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

fix(server): setup scripts complete with shell line editors - #25472

Open
maria-rcks wants to merge 1 commit into
mainfrom
maria/takeover-pr-17817
Open

maria-rcks wants to merge 1 commit into
mainfrom
maria/takeover-pr-17817

Conversation

@maria-rcks

@maria-rcks maria-rcks commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Bash line editors can treat carriage-return-separated setup wrappers as unfinished multiline input. Worktree setup then never reports completion and the waiting agent cannot start.

Change

Submit complete POSIX commands with encoded script text, preserve trailing newlines, and evaluate in a subshell before reporting exit status. Long scripts use bounded, independently complete assignments. Preserve Fish, PowerShell, and main's startup-readiness handling. Reuse the existing test harness instead of adding another mock runner.

Scope and approval

This is an obvious, focused bug fix under the small-fix contribution exception. It only repairs POSIX completion-observed setup and settle delivery, preserving the original intent and all other shell paths. Maintainer takeover was already recorded on https://git.cafe/pingdotgg/t3code/pulls/17817, and Maria explicitly requested this replacement after the archived fork rejected pushes.

Verification

  • vp run check:changed passed through Blacksmith: format, lint, both knip passes, GraphQL generation, affected typechecks, and all 12 adjacent tests. The six shell cases include literals/heredocs, long input, stdin, trailing backslash-newline, explicit exit, and syntax error. Two fail against current main before the fix.
  • Real dev client with Bash 5.2 and ble.sh: main remains in MULTILINE with setup waiting; this branch completes the same setup and the Codex agent replies setup verified.
  • Additional real node-pty checks pass with plain Bash and ble.sh: multiline/heredoc, trailing backslash-newline, explicit exit 7, syntax error 2, and 120-line Unicode script.
  • Two independent GPT-6.1-Sol static reviews found no blockers. The replacement's file contents are identical to the reviewed merge candidate.
  • Unverified: native zsh, Fish, PowerShell, macOS/Windows, mobile/Electron, relay/tunnel. Long-input byte limits are tested; the long case was not repeated through the browser. Existing cosmetic limit: line-editor redraws can show internal assignments for long scripts.

main waits in ble.sh multiline input

replacement completes setup and starts the agent

worktree setup completes and starts the agent

Takeover

Replaces https://git.cafe/pingdotgg/t3code/pulls/17817, with thanks to @sgaabdu4 for the original implementation.

The contributor fork is archived and rejects maintainer pushes. Compared with the original PR's current diff, this retains its production wrapper and all six shell regression cases, replaces the duplicate test harness with main's harness, and preserves main's startup-readiness tests. No behavior was dropped. The commit credits Abid A Gafoor.

Built with GPT-6.1-Sol in the Codex harness on T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 26, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 4e318cc

Macroscope's review found this PR approvable — Updates POSIX setup-script completion handling to work with shell line editors and long commands. Existing script selection and execution restrictions remain unchanged, keeping the change narrowly scoped.

No code changes detected at 8c07e7f. Prior analysis still applies.

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

@github-actions

github-actions Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 444fdb0 · PR result: 8c07e7f · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 4e318cc and 8c07e7f.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The POSIX completion wrapper encodes and evaluates setup commands in a subshell, splits long commands into terminal-sized chunks, and reports their exit status. Tests cover multiline scripts, long commands, stdin, trailing backslash-newline behavior, and exit codes.

Changes: POSIX setup command delivery

Layer / File(s) Summary
POSIX wrapper and execution validation
apps/server/src/project/ProjectSetupScriptRunner.ts, apps/server/src/project/ProjectSetupScriptRunner.test.ts
The wrapper encodes and evaluates commands in a subshell, splits commands that exceed the 1,023-byte limit, and emits the command status. Live tests run submitted commands through Bash and check multiline syntax, literal text, long scripts, stdin, trailing backslash-newline behavior, and exit codes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Possibly related PRs

  • pingdotgg/t3code#24775: Both PRs change setup command delivery in ProjectSetupScriptRunner; this PR changes the POSIX wrapper sent after shell readiness.
  • pingdotgg/t3code#23576: Both PRs modify the shell-specific completion behavior in ProjectSetupScriptRunner.
  • pingdotgg/t3code#24428: Both PRs modify setup-script completion wrappers and exit-status reporting.

Suggested labels: bugfix

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: Minimal · previous review 4e318

No demonstrated issue prevents merging after normal checks; the reported interactive-shell checks remain unverified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: setup scripts now complete when a shell line editor (Bash with ble.sh) is in use. It matches the changeset, which changes the POSIX setup wrapper so that submitted com…
Description check Passed The description follows the repository template. It has Problem, Change, Scope and approval, and Verification sections, plus a takeover note. The Problem section gives the bug and its effect, and the …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main change: setup scripts now complete when a shell line editor (Bash with ble.sh) is in use. It matches the changeset, which changes the POSIX setup wrapper so that submitted commands complete, and it follows the conventional commit format. The title is concise and specific.

Full details: Description check

Explanation

The description follows the repository template. It has Problem, Change, Scope and approval, and Verification sections, plus a takeover note. The Problem section gives the bug and its effect, and the Change section explains the fix. Scope cites the small-fix exception and the prior takeover PR. Verification lists the checks run, the observed results, and what was not verified, including zsh, Fish, PowerShell, macOS/Windows, and mobile/Electron. The template's agent-model line is present at the end. The description covers what the template requires.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 301c1a1 and 4e318cc.

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; 7 remain after this review.

Copy link
Copy Markdown
Collaborator Author

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 301c1a1a30b00a7ef3bae145ef3c6c310a0f8ff7 on this linux boat vm. git.cafe and github both use https://git.cafe/pingdotgg/t3code/pulls/25472. the pr is open; its sole commit has no authors from the excluded maintainer accounts.

  • installed dependencies with vp i in a separate github/main worktree (4914cdd98f) and on the exact pr head. ran both dev environments in the background with separate state. used repository playwright and /usr/bin/google-chrome.
  • created a project pointing at a real git repository through the client. configured a synchronous worktree setup action through the action editor. used bash 5.2.21 with ble.sh cb9c2c3.
  • on main, the multiline script with an apostrophe, quoted heredoc, unicode, and a trailing comment stays in ble.sh's MULTILINE state. setup waits and the provider does not start.
  • on this head, the same script completes, creates setup-proof.txt, and starts a real codex gpt-6.1-sol turn through the local proxy. codex runs cat setup-proof.txt and returns setup verified.
  • neighboring behavior: a 120-line unicode/quoted/backslash script completes through the same worktree flow, then codex reads its proof file. exit 7 reports exactly that status in the client and prevents the agent from starting.
  • blacksmith vp run check:changed 071239804e375078dd3fae3b14704b4def57e048 passed format, lint, both knip passes, graphql generation, affected typechecks, and all 12 adjacent tests. an explicit pr base was needed because the worker has shallow history. the pr currently has 29 successful checks, none failing or pending.

short runtime transcript:

$ git rev-parse HEAD
301c1a1a30b00a7ef3bae145ef3c6c310a0f8ff7
$ cat .t3/worktrees/setup-verification/t3-795aa242/setup-proof.txt
setup verified
$ cat .t3/worktrees/setup-verification/t3-d20de44d/setup-proof.txt
setup verified
client, exit-7 scenario: Setup script exited with 7.
blacksmith adjacent tests: 12 passed

main: ble.sh multiline setup blocks agent startup

pr: codex reads the file created by the same setup

pr: setup completes and the real codex turn reads its proof file

pr: exit 7 is reported and agent startup is blocked

Not verified

native ios/android and electron clients, macos/windows, zsh/fish/powershell, relay/tunnel connections, other providers, and the settle-trigger path were not exercised on this linux web-client pass. interactive stdin and trailing backslash-newline have passing adjacent automated coverage, but were not repeated through the browser. no performance claim is made; ble.sh redraws expose internal assignments during long scripts.

GigaZoro pushed a commit to GigaZoro/CodeShip that referenced this pull request Oct 30, 2026
…ingdotgg#25472)

Cherry-picked from pingdotgg#25472

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GigaZoro pushed a commit to GigaZoro/CodeShip that referenced this pull request Oct 30, 2026
Upstream pingdotgg#25472 runs setup scripts through eval so shell line editors complete them. macOS /bin/bash 3.2 exits 1 for a syntax error inside eval where bash 4+ exits 2; both still report a failed setup.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AkiVonAkira

Copy link
Copy Markdown
Contributor

Note

🤖 Claude Opus 5.5 responding on behalf of Kayzel

Reproduced on T3 Code 0.1.7-nightly.20261102.4061 (server on Linux, Bash 5.3.9, ble.sh 0.4.0-nightly+fb1811e). A worktree setup script stayed in ble.sh's -- MULTILINE -- (RET or C-m: insert a newline, C-j: run) state, the step sat on "Seed Unity Library" for several minutes, and the agent never started. The script itself runs in about 10 ms when invoked directly.

Typing the current POSIX wrapper (( cmd\r); printf '\n__T3_SETUP_DONE__%s\n' "$?"\r) into an interactive Bash with ble.sh attached reproduces it. With ble.sh left detached, the same input prints the sentinel. I'm working around it by skipping ble-attach when NO_COLOR=1 and FORCE_COLOR=0, which only setup terminals set. I'd like to drop that once this lands.

Co-authored-by: Abid A Gafoor <40262245+sgaabdu4@users.noreply.github.com>
@maria-rcks
maria-rcks force-pushed the maria/takeover-pr-17817 branch from 4e318cc to 8c07e7f Compare November 6, 2026 04:43

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:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants