Skip to content

fix(server): preserve filename whitespace when reading and saving files - #29314

Open
voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:voltcrash/preserve-file-path-whitespace
Open

voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:voltcrash/preserve-file-path-whitespace

Conversation

@voltcrash

@voltcrash voltcrash commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes https://git.cafe/pingdotgg/t3code/issues/29313

Reading "report.txt " currently returns report.txt; saving it overwrites that other file. Leading whitespace fails the same way. Preserve filename whitespace through the read/write RPC inputs, results, errors, and server path resolution. Empty paths and the existing length limits, workspace confinement, symlink checks, and Git-metadata write guards still apply.

This fits the focused obvious-bug exception: one valid filename silently targets another file, risking data loss. It fixes read/save targeting, separately from the duplicate file-tree entries in closed https://git.cafe/pingdotgg/t3code/pulls/22847. No UI or workspace-root schema changes.

Proof on Linux / Node v26.12.1:

  • Existing read/save cases extended with RPC codec round trips failed first: wrong contents returned, ordinary file overwritten. The absolute-read extension failed at its separate trim. All pass after the fix, including leading/trailing spaces, tabs, and a whitespace-only filename; traversal is still rejected.
  • Local review found the same trimming in error payloads. An assertion added to the existing structured-error case failed first on the whitespace-only filename; all 6 contract tests pass after correction.
  • Focused service/consumer suites: 107 tests passed across file operations, path resolution, contracts, directory listing, assets, and favicons. vp run check:changed: all stages passed (format, lint, knip, generated GraphQL, typechecks for 35 affected/dependent packages, 37 neighboring tests).
  • Actual service runtime after the fix (before: both reads returned ordinary file, and saving overwrote it):
{"requested":"report.txt ","directRead":"whitespace file","rpcRead":"whitespace file","ordinaryAfterSave":"ordinary file","spacedAfterSave":"edited"}
{"requested":" report.txt","directRead":"whitespace file","rpcRead":"whitespace file","ordinaryAfterSave":"ordinary file","spacedAfterSave":"edited"}

No native Windows/macOS run or UI claim; unsupported whitespace filename forms are excluded from Windows test rows.

Reviewed with review-loop: 2 rounds with Claude Opus 5.5 on xhigh. Reviewer verdict: approved.

Created with GPT-6.1 Sol in Codex, running in T3 Code.

Fixes #29313

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Nov 2, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 9e3b8c1

Macroscope's review found this PR approvable — File reads and saves now preserve filename whitespace instead of targeting a trimmed name. Existing path-containment and file-access safeguards remain unchanged, keeping the change narrowly scoped.

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

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Read and write path schemas now accept non-empty paths with surrounding whitespace. Workspace path handling preserves the original path during read targeting and relative-path resolution. Tests cover whitespace-containing filenames, schema round trips, and traversal rejection.

Changes: Whitespace-preserving file paths

Layer / File(s) Summary
Read and write path contracts
packages/contracts/src/project.ts, packages/contracts/src/project.test.ts
Read and write input, result, and error schemas accept non-empty path values without trimming whitespace. Tests verify that error paths with spaces survive encoding and decoding.
Workspace path preservation
apps/server/src/workspace/WorkspacePaths.ts, apps/server/src/workspace/WorkspaceFileSystem.ts, apps/server/src/workspace/WorkspaceFileSystem.test.ts
Workspace path resolution and read targeting use the original path. Tests check whitespace-containing reads and writes, confirm that report.txt remains unchanged during a whitespace-path write, and verify that traversal paths fail with WorkspacePathOutsideRootError.

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

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • pingdotgg/t3code#26254: Both changes involve resolveReadTarget; this change preserves whitespace in paths handled by that resolver.

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: Minimal · up to ef73e

No actionable issue remains in the reviewed changes; the PR is mergeable after normal checks.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title describes the main change: preserving filename whitespace during reads and saves.
Linked Issues check Passed [#29313] Read and write schemas preserve path whitespace through RPC inputs and results. Server path resolution uses the original path. Tests cover leading and trailing whitespace, codec round trips, …
Out of Scope Changes check Passed [#29313] The contract and server changes support correct read and write targeting. The error-path changes preserve the same paths in error payloads. The tests cover this behavior and path confinement.…
Description check Passed The description explains the problem and expected behavior, describes the focused fix and scope, and gives specific verification results and limitations. It also explains why the obvious-bug exception…
Full details: Linked Issues check

Explanation

[#29313] Read and write schemas preserve path whitespace through RPC inputs and results. Server path resolution uses the original path. Tests cover leading and trailing whitespace, codec round trips, and writes that leave the ordinary file unchanged. Closed #22847 is background only.

Full details: Out of Scope Changes check

Explanation

[#29313] The contract and server changes support correct read and write targeting. The error-path changes preserve the same paths in error payloads. The tests cover this behavior and path confinement. The summary shows no unrelated changes.

Full details: Description check

Explanation

The description explains the problem and expected behavior, describes the focused fix and scope, and gives specific verification results and limitations. It also explains why the obvious-bug exception applies and identifies the model and harness used.

Finishing Touches

Generate unit tests (beta)

  • 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 9e3b8c1 and ef73ea0.

Files selected for processing: 2

  • apps/server/src/workspace/WorkspaceFileSystem.test.ts
  • packages/contracts/src/project.ts

Limit details: You’ve used all 10 included reviews currently available.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed labels Nov 2, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 9e3b8c1d942e6bc9a83e71d054790b8ca3368659 on linux with the real dev server, repository Playwright, /usr/bin/google-chrome, and real codex gpt-6.1-sol turns. added a project and threads through the app; codex created the files in a separate real git repository. ran one dev server at a time.

  • reproduced on github/main (a036489016) first: whitespace-path reads returned the ordinary file; saving report.txt overwrote report.txt.
  • on this head, chat file links opened the distinct leading-space, trailing-space, and tab-wrapped files. edited and autosaved both space variants in the real editor, switched files, and reopened them. disk contents and websocket requests confirmed the exact paths, while report.txt stayed unchanged.
  • authenticated websocket read/write round trips also passed for a whitespace-only path (" "), absolute reads, optional missing files, and whitespace-preserving error payloads.
  • nearby behavior passed: normal markdown source edits and rendered task-checkbox saves persisted; traversal and .git/config writes were rejected.
  • blacksmith ran vp test run apps/server/src/workspace/WorkspaceFileSystem.test.ts packages/contracts/src/project.test.ts: 2 files, 33 tests passed. all 33 reported ci checks were green. no source changes or pushes were needed.

runtime transcript

main: clicking the trailing-space file link shows the ordinary file.

main reads the ordinary file for the trailing-space filename

pr: the same filename opens its distinct contents.

pr reads the correct trailing-space file

recording: edit the leading-space file, switch to the saved trailing-space file, check the unchanged ordinary file, then reopen the leading-space edit.

real editor saves whitespace filenames and preserves the ordinary file

Not verified

native desktop, ios/android, macos/windows filesystem behavior, other providers, and relay/tunnel connections were not exercised on this linux vm.

the existing file-tree whitespace normalization is outside this pr's scope: variants collapse into one tree entry, and a whitespace-only basename caused a folder-list encoding failure on main. this verification used actual chat file links for editor access and websocket calls for the whitespace-only path; it does not claim that tree selection is fixed. main also hit a whitespace-only checkpoint encoding failure; after moving that case into a subfolder through the app terminal, the pr's real codex turn and checkpoint completed.

@voltcrash
voltcrash force-pushed the voltcrash/preserve-file-path-whitespace branch from 9e3b8c1 to ef73ea0 Compare November 7, 2026 09:52

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

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed 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.

[Bug]: Reading and saving filenames with whitespace targets another file

3 participants