镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(server): worktree setup no longer hangs when the shell is Nushell - #29461

Open
KenDJohnson wants to merge 4 commits into
pingdotgg:mainfrom
KenDJohnson:t3/fix-nushell-setup-status
Open

KenDJohnson wants to merge 4 commits into
pingdotgg:mainfrom
KenDJohnson:t3/fix-nushell-setup-status

Conversation

@KenDJohnson

@KenDJohnson KenDJohnson commented Nov 2, 2026 •

Copy link
Copy Markdown

Problem

When the setup terminal's shell is Nushell, worktree setup and settle actions never finish. T3 types a POSIX wrapper, ( <command> ); printf '\n<sentinel>%s\n' "$?", and nu passes "$?" to printf literally, so the terminal prints __T3_SETUP_DONE___<token>:$?, which never matches the numeric completion pattern. When the command fails, nu stops before printf and prints nothing. Either way, a synchronous setup holds the thread forever.

Fixes https://git.cafe/pingdotgg/t3code/issues/24423 (reported on Linux; reproduced here on macOS with T3 Code v0.1.6 and Nushell 0.116.1).

Change

ProjectSetupScriptRunner now treats nu as its own completion shell and wraps the action in Nushell syntax:

let __t3c = try {
<command>
0 } catch {|e| if $e.exit_code? == null { print -e $e.rendered }; $e.exit_code? | default 1 }; print $"\n<sentinel>($__t3c)"

Nu skips the rest of the input after a failing command, so the status print only runs behind try. A failing external reports its own exit code ($e.exit_code). Other errors (command not found, error make) report 1 and are printed, since catch would otherwise hide them.

Like zsh and fish, nu's line editor enables bracketed paste, so the command is still typed only after that signal. The readiness check now names the shells that lack the signal (PowerShell, cmd) instead of the ones that have it.

Scope and approval

https://git.cafe/pingdotgg/t3code/issues/24423 reports this bug; it has not been triaged yet. This is a small, focused fix for an obvious bug: one new case in the existing shell switch, plus keeping nu on the existing line-editor wait. Setup and settle actions, including thread launches from MCP and scheduled tasks, all go through this runner. No contract, client, or docs changes are needed.

Verification

  • A new case in ProjectSetupScriptRunner.test.ts checks that nu waits for its line editor and receives the Nushell wrapper. It fails on main, where nu gets ( vp i\r); printf … "$?", and passes here. Reverting only the readiness change also fails it, because the command is typed before the line editor starts. vp run check:changed passes.
  • Real Nushell 0.116.1 in a PTY on macOS, typing the generated wrapper into an interactive login shell with a full user config:
    • echo setup-ok → 0; node -e "process.exit(7)" → 7
    • unknown command → 1, with nu's error shown; error make {msg: "boom"} → 1
    • multi-line commands and a trailing # comment → 0
    • external stdout stays a TTY
  • End to end on a dev server (macOS, SHELL set to a Nushell login wrapper, Replay provider), with a synchronous setup action on a scrap repo, via t3 thread start --worktree main --wait:
main    echo setup-ok               setup terminal: __T3_SETUP_DONE___767656c2…:$?   thread start still waiting at the 45s timeout
branch  echo setup-ok               setup terminal: __T3_SETUP_DONE___45e5f29b…:0    replay turn runs, CLI exits 0
branch  node -e "process.exit(3)"   "stopped with status failed: … Setup script exited with 3."   CLI exits 1

Not checked: Nushell on Linux or Windows, and Nushell versions older than 0.116. Setup actions must still be valid Nushell: POSIX-only syntax such as a && b fails to parse, so no status is printed, the same as a syntax error in a POSIX shell.

Model and harness: Claude Opus 5.5 through Claude Code in T3 Code.

Fixes #24423

Setup and settle actions typed a POSIX completion wrapper into Nushell,
which passes "$?" to printf literally, so the completion marker never
carried an exit code and synchronous setup waited forever. Nushell now
gets its own wrapper that reports the exit code through try/catch.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 62528d0

Macroscope's review found this PR approvable — Adds Nushell-compatible completion and error reporting for existing project setup scripts, preventing completion tracking from hanging. The change leaves script selection, execution triggers, and other shells’ behavior unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The setup runner recognizes Nushell, waits for its readiness marker, and wraps setup commands with Nushell-specific completion and exit-status handling. Tests check readiness and wrapper output for default and special-character commands.

Changes: Nushell setup completion

Layer / File(s) Summary
Nushell completion flow
apps/server/src/project/ProjectSetupScriptRunner.ts, apps/server/src/project/ProjectSetupScriptRunner.test.ts
The runner maps nu to Nushell and waits for shell readiness before writing a Nushell-specific wrapper. The wrapper runs the command from the worktree path, reports its exit status with a completion sentinel, and removes its temporary script file when possible. Tests check readiness and escaping for default and special-character commands.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

  • Issue 24423: Reports that Nushell setup commands receive a POSIX completion wrapper. This change adds Nushell-specific completion handling and exit-status reporting.

Possibly related PRs

  • pingdotgg/t3code#25472: Both changes modify completion delivery in ProjectSetupScriptRunner; this change adds Nushell wrapping and readiness handling.
  • pingdotgg/t3code#24428: Both changes implement Nushell completion handling in ProjectSetupScriptRunner.
  • pingdotgg/t3code#24775: Both changes wait for shell readiness before sending setup commands.

Suggested reviewers: macroscopeapp

Merge Risk: Minimal · up to 4f7dc

No merge-blocking risk was identified for Nushell setup.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title is concise, specific, and accurately describes the primary fix: preventing worktree setup hangs when the shell is Nushell.
Linked Issues check Passed Objective [#24423]: a Nushell setup terminal must run the setup command, record its numeric exit status, and must not wait for a marker that never matches.

Evidence at the reviewed head (merge base e…
Out of Scope Changes check Passed All changed hunks connect to the Nushell completion fix for #24423:
- ProjectSetupScriptRunner.ts: the nu detection case, the nushell wrapper case, the readiness condition, and the bracketed-pas…
Description check Passed The description covers the problem, the Nushell-specific change, scope and approval rationale, and focused verification results. It also states the checks that were not performed and identifies the mo…
Full details: Linked Issues check

Explanation

Objective [#24423]: a Nushell setup terminal must run the setup command, record its numeric exit status, and must not wait for a marker that never matches.

Evidence at the reviewed head (merge base edc37c9 to head 4f7dc60):

  • resolveCompletionShell maps nu to a new nushell case. Before this PR, nu mapped to posix.
  • wrapCommandForCompletion, nushell case: the wrapper sets $env.LAST_EXIT_CODE = 0. It creates a temporary .nu file in $env.T3CODE_WORKTREE_PATH with mktemp. It writes the escaped command to that file, with a leading rm --permanent line. It runs ^$nu.current-exe --login --commands with cd $env.T3CODE_WORKTREE_PATH; source &lt;file&gt;. A catch sets LAST_EXIT_CODE to 1 when it is still 0. The wrapper then removes the file and prints \n&lt;sentinel&gt;($__t3c). The completion pattern accepts this numeric value. A failing external command therefore does not stop the final print.
  • The command text is escaped for a Nu double-quoted string. Quotes, backslashes, and control characters 0x00-0x1f are escaped. Newlines are passed to the file as real line breaks.
  • Readiness: the line-editor wait now applies to every shell except powershell and null. Nushell waits for bracketed paste before typing. posix and fish keep the wait.
  • ProjectSetupScriptRunner.test.ts adds a table-driven Nushell case. It checks that nothing is typed before \x1b[?2004h. It then checks the exact wrapper text, including multi-line, quoted, and control-character input. The harness and setup helper accept an optional command, defaulting to vp i, so existing tests keep their behavior.

The current test matches the current wrapper, which uses the temp-file form. The previous review described an older nested-process form. I used the head code as the current evidence.

Not verified by me: runtime behavior of mktemp, rm --permanent, source, nested exit-code propagation, and the $env.LAST_EXIT_CODE update on external failure. The unit test confirms emitted text and the readiness wait, not Nushell runtime results. The PR reports macOS PTY and end-to-end runs, and a comment reports a Linux VM run. I did not reproduce those runs.

The optional explicit project-script shell in #24423 is an alternative, not a requirement. The PR objectives state that it is not in scope.

Full details: Out of Scope Changes check

Explanation

All changed hunks connect to the Nushell completion fix for #24423:

  • ProjectSetupScriptRunner.ts: the nu detection case, the nushell wrapper case, the readiness condition, and the bracketed-paste comment that lists nu.
  • ProjectSetupScriptRunner.test.ts: the optional command parameter on the harness and setup helper, which supports the new Nushell test cases, and the new Nushell test cases.

The temporary .nu file in the worktree is part of the Nushell wrapper. The PR objectives say it keeps relative source and use imports resolved from the worktree when login.nu changes the directory. It is removed before the action runs, and the wrapper removes it on preparation errors. This connects to correct Nushell setup execution in the worktree, so it is not unrelated scope.

No changes to contracts, clients, docs, or unrelated server modules appear in the reviewed diffs for these files. The summary and the commit messages do not show other changed paths. Those other paths were not inspected here.

Full details: Description check

Explanation

The description covers the problem, the Nushell-specific change, scope and approval rationale, and focused verification results. It also states the checks that were not performed and identifies the model and harness.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ast-grep (0.51.1)

ast-grep could not parse rule config: /ast-grep-rules/ast-grep-essentials/rules/typescript/security/log-injection-typescript.yml

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 7f9ac59 and 4f7dc60.

Files selected for processing: 2

  • apps/server/src/project/ProjectSetupScriptRunner.test.ts
  • apps/server/src/project/ProjectSetupScriptRunner.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Run observed scripts in a child of the current Nushell executable, loading
its login configuration. The parent reports LAST_EXIT_CODE after the child
finishes, including explicit exit commands, without relying on catch-record
fields that differ between Nushell versions.

Extend the existing mocked-terminal test to assert the complete wrapper,
including the failure fallback, sentinel, and quoting of multiline input.
The expanded test fails before the change.

Validation: vp run check:changed passes. Real PTY verification passed 40
cases across Nu 0.97.1, 0.98.0, 0.99.1, and 0.116.1 on macOS:

  echo setup-ok                     -> completion sentinel:0
  node -e "process.exit(7)"          -> completion sentinel:7
  open /t3-review-does-not-exist     -> error shown, completion sentinel:1
  error make {msg: "boom"}           -> error shown, completion sentinel:1
  exit 0                            -> completion sentinel:0
  exit 7                            -> completion sentinel:7

Configured commands, multiline input, quoting, and stdin/stdout/stderr TTY
behavior also passed on each version.

Model and harness: GPT-6.1 Sol through Codex in T3 Code.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

@maria-rcks

maria-rcks commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified on a linux boat vm with nushell 0.116.1, the dev web client in /usr/bin/google-chrome driven by the repo's playwright, and real codex gpt-6.1-sol turns. created a real git project, imported its synchronous setup action through the client, and launched worktree threads through the running server's t3 thread start --worktree main --wait path. no mocks, fixtures, or replay provider.

  • github/main at c3cfb5e1e3: setup printed setup-ok and returned to the prompt, but the completion marker ended in literal $?. the client remained on "setting up worktree" and codex did not start. stopped the baseline after reproducing this failure.
  • original pr head 31a7e9e00d: normal setup completed and codex read the output file. reproduced the review's directory bug with login.nu changing directory: setup reported success but wrote outside the worktree, and codex found the file missing.
  • fixed and pushed 7f9ac5916e for login-directory changes. further runtime verification found parser-time source resolution still used the login directory. the contributor addressed that in 4f7dc60f184e840e6bcc4c6169c411dee8b1b96d, which is the verified final head.
  • final head: with login.nu changing directory and a conflicting setup.nu in that directory, source ./setup.nu and use ./module.nu module-ok load the worktree's files. a command defined in config.nu also works. codex reads source-cwd.txt, and its contents match the new worktree and provider cwd.
  • neighboring behavior: exit 7 fails setup with status 7; error make fails with status 1. neither starts codex. multiline input, quotes, and a trailing comment complete with status 0. settling through the client runs the settle action and emits status 0.
  • setup runs git add -A: only source-cwd.txt is staged, with no .t3-setup-* file staged or left behind. the temporary script deletes itself before user commands run.
  • exact final wrapper also exercised directly with nushell 0.97.1: relative source/module imports succeed, exit 7 emits 7, and a native error emits 1. full app verification used 0.116.1.
  • blacksmith on the final head: vp run check:changed edc37c96c810d42761ab510c6a3e9d0a741547dc passed format, lint, knip, graphql, affected typechecks, and 12 tests in 2 files. cafe reports 28 successful checks and none pending or failing.

short runtime transcript:

main: setup-ok; __T3_SETUP_DONE___<token>:$?; thread remains preparing
final setup: source-from-worktree; module-from-worktree; configured-command
final setup: source-cwd.txt; quoted; __T3_SETUP_DONE___<token>:0
codex: source-cwd.txt and cwd = .../worktrees/nu-verification/verify-latest-source
codex: provider-ok
exit 7: Workspace preparation failed ... Setup script exited with 7.
error make: Workspace preparation failed ... Setup script exited with 1.
client settle: settled=true; __T3_SETUP_DONE___<token>:0
staged files: source-cwd.txt

before on main:

main hangs after successful nushell setup

after on the final head:

real codex reads setup output from the correct worktree and client settles

real codex turn completes and the client settles the thread

inspected the screenshots and recording, and exercised uploaded video playback and seeking.

Not verified

native desktop, ios, android, macos, windows, and scheduled-task launches were not exercised on this linux vm. older nushell received direct wrapper checks rather than a full client/provider pass. no performance comparison is claimed.

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Nushell parses --commands before evaluating its leading cd. If login.nu
changes directories, relative source and use statements can select files
from the login directory even when the action runs in the worktree.

Use Nu's mktemp, save, to nuon, source, and rm commands to load the action
from a temporary .nu file in the worktree. Relative imports resolve against
that file's directory, and the child retains the user's login definitions.
Remove the file before the action runs so it does not appear in git status;
clean it up in the parent when parsing fails. Preparation errors still
report a numeric completion status. Guard null catch arguments on older Nu.

Extend the existing mocked-terminal table with source/use input and assert
the complete native wrapper. The expanded assertion fails before the fix.
vp run check:changed passes, including 12 tests in two affected files.

Runtime: 60 PTY cases and eight preparation-failure cases passed on macOS
with Nushell 0.97.1, 0.98.0, 0.99.1, and 0.116.1. Login startup changes cwd;
both directories contain conflicting setup.nu and module.nu files:

  before: source ./setup.nu           -> SOURCE-WRONG, status 0
  before: use ./module.nu *; location -> MODULE-WRONG, status 0
  after:  source ./setup.nu           -> SOURCE-WORKTREE, status 0
  after:  use ./module.nu *; location -> MODULE-WORKTREE, status 0
  file present only in login cwd     -> diagnostic, status 1, no leftover
  exit 0 / exit 7 / error make        -> statuses 0 / 7 / 1

Nested imports, login-defined commands, multiline quoting, paths with spaces
and quotes, TTY input/output, and cleanup passed. Creation and save errors
show diagnostics and report status 1 without running the action.

Model and harness: GPT-6.1 Sol through Codex in T3 Code.
@KenDJohnson

Copy link
Copy Markdown
Author

@maria-rcks thanks for the review and tests on this, anything else that needs to happen here?

This branch has not been deployed

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

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Worktree setup fails with Nushell because completion tracking uses POSIX syntax

2 participants