[only21mil]

Problem

If the T3 home is reached through a symlink, "Delete old rotated logs" and "Delete old browser artifacts" never delete anything. Examples are ~/.t3 linked to a bigger disk, T3CODE_HOME on a linked volume, or a home under macOS /tmp or /var. Every run reports "No expired files" while server.log.N files and captures keep piling up.

cleanFiles in apps/server/src/storageCleanup.ts returns early when realPath(root) differs from path.resolve(root). Any symlink anywhere above the folder triggers that, not only a logs folder that is itself a link.

Change

The check now compares the folder's real path with its real parent plus its own name. Worktree cleanup in the same file already uses this rule, from #16231.

• A linked T3 home or any other linked ancestor is now cleaned.
• A logs or artifacts folder that is itself a symlink is still skipped, as before.
• The walk still starts at the real root and still skips every entry whose real path differs from its own path, so links inside the folder are never followed.

Three production lines changed. One it.live.each test covers a symlinked home (removed) and a symlinked logs folder (kept). The second case also passes on main.

Scope and approval

#19532 reported storage cleanup tests failing under the macOS temp dir. The maintainer triage approved resolving generated test directories only; #21442 made that test-only fix. This PR addresses a separate, reproduced production failure: enabling cleanup on a linked T3 home keeps every expired file. It qualifies as a very small, focused fix of an obvious bug: the folder check rejects linked ancestors, and the fix uses the existing worktree-cleanup rule in this file while retaining protection for a linked cleanup folder. It changes no settings or defaults.

Verification

Regression test, on main 05696c636e plus only the new test (Linux x64):

pnpm exec vp test run apps/server/src/storageCleanup.test.ts: "rotated logs behind a symlinked home are removed" fails. It expected removed / Removed 1 rotated logs and got kept / No expired files. The logs-folder case passes.

This branch merged with main 5ffe045931:

• Linux x64: both new cases pass; 119 of 120 tests pass. The one failure, "compares the checked-out branch without a full status", fails the same way on main with that host's git 2.56 (gitfile does not point to a valid repository). It is worktree code this change does not touch.
• macOS arm64: 120 of 120 pass.
• vp lint and vp fmt --check on both changed files, and pnpm run typecheck in apps/server: clean.

Real app, Linux, production builds of main 05696c636e and this branch 2d27f1164a, in the browser and in the desktop app. Each run used an isolated T3 home t3home -> disk/t3home:

1. Settings > Storage: turn on "Delete old rotated logs" (8 days).
2. Write userdata/logs/server.log.1 with a modified time 10 days back.
3. Click "Delete now" and open "Last run".

Browser, server started with --base-dir t3home. On main, the report says "No expired files" (0 removed, 1 kept) and the file is still on disk.

On this branch, the server's startup sweep removed the old file first. I wrote it again and clicked "Delete now". The report says "Removed 1 rotated logs" (1 file, 16 B) and the file is gone.

Desktop app, launched with T3CODE_HOME=t3home, a fresh home per build. On main, the report again says "No expired files" and the file stays.

On this branch, "Delete now" removes it: "Removed 1 rotated logs" (1 file, 16 B), and the file is gone.

Not checked:

• Windows, where the link would be a junction.
• Browser artifacts in the real app. They go through the same cleanFiles call, but only logs were exercised.
• The macOS desktop app.

Prior PRs

• fix(server): resolve the generated test home in ServerConfig.layerTest #19540 by @ScottN-PV and fix(server): storage cleanup tests pass with the macOS default TMPDIR #19541 by @whoisaldo resolved the test temp dir for [Bug]: storageCleanup test fails on macOS with the default TMPDIR #19532. Both were closed in PR triage as test maintenance. fix(server): server tests resolve the macOS temp dir symlink #21442 by @nabettu made the same test-only fix and was merged. None of them changed the product guard.
• [Bug]: storageCleanup test fails on macOS with the default TMPDIR #19532 was reported by @nbrosei. The guard itself came from feat(settings): add automatic storage cleanup per machine and project #11598 by @maria-rcks.

Both closures required maintainer agreement for test maintenance (#19540, #19541). This PR reuses none of their code.

Production fix, tests and images written with Claude Opus 5.5; PR description updated with GPT-6.1 Sol and Claude Opus 5.5 in T3 Code Nightly.

🤖 Generated with Claude Opus 5.5 and GPT-6.1 Sol in T3 Code Nightly

[macroscopeapp]

Approvability

Verdict: Approved at 2d27f11

Macroscope's review found this PR approvable — This is a narrowly scoped server bug fix that restores configured log and artifact cleanup when the T3 home has a symlinked ancestor, while retaining protections against following symlinked cleanup folders and entries. Regression coverage verifies both the newly supported and still-rejected cases.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Warning

Review limit reached

Only developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing.

Next included review available in 31 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: c3888da
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 217321a4-bca4-4c0b-b03a-6592ebe4f77a

Commits

Reviewing files that changed from the base of the PR and between 2d27f11 and 90e8033.

Files selected for processing: 2

• apps/server/src/storageCleanup.test.ts
• apps/server/src/storageCleanup.ts

No actionable comments were generated in the recent review.

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: fbc6a08
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 45b452bb-6d97-43b3-8d8a-a2181e85647c

📥 Commits

Reviewing files that changed from the base of the PR and between fbc6a08 and 2d27f11.

📒 Files selected for processing: 2

• apps/server/src/storageCleanup.test.ts
• apps/server/src/storageCleanup.ts

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

Walkthrough

cleanFiles now allows a cleanup root whose parent resolves through a symlink, but still rejects a root that is itself a symlink. Added tests cover rotated-log cleanup for both path setups.

Changes: Cleanup root validation

Layer / File(s)	Summary
Validate cleanup roots and test symlinked paths apps/server/src/storageCleanup.ts, apps/server/src/storageCleanup.test.ts	cleanFiles allows a symlinked parent while rejecting a symlinked cleanup root. Tests check that cleanup removes an expired rotated log through a symlinked home directory, but keeps it when the logs directory itself is symlinked.

Priority: Normal

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

Change: Bug fix

Possibly related PRs

• pingdotgg/t3code#24061: Both PRs modify storage cleanup safety in apps/server/src/storageCleanup.ts and use real-path checks, but they guard different cleanup targets.

Suggested labels: size:S

Suggested reviewers: maria-rcks

Merge Risk: Minimal · up to 2d27f

No actionable merge-blocking cleanup risk remains; the change is ready to merge after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is specific and matches the main change. The diff changes cleanFiles in storageCleanup.ts so cleanup works when the T3 home or an ancestor is symlinked, and it keeps skipping a cleanup folde…
Description check		The description follows the template closely. It has Problem, Change, Scope and approval, and Verification sections. The Problem section explains the bug and its trigger. The Change section explains t…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title is specific and matches the main change. The diff changes cleanFiles in storageCleanup.ts so cleanup works when the T3 home or an ancestor is symlinked, and it keeps skipping a cleanup folder that is itself a symlink. The title names both the fix and the affected areas (old logs and artifacts) under a symlinked T3 home. It uses the conventional commit format from the template. The title is concise and free of file lists or vague wording.

Full details: Description check

Explanation

The description follows the template closely. It has Problem, Change, Scope and approval, and Verification sections. The Problem section explains the bug and its trigger. The Change section explains the new comparison rule and the protections that remain. The Scope and approval section cites the related issue and maintainer triage, and explains why the change qualifies as a small focused fix without a prior approval. The Verification section lists the regression test, platform results, lint and typecheck, and real-app checks, and it states what was not checked (Windows, browser artifacts in the real app, macOS desktop app). It also names the models used, as the template requests. Minor issues: the Prior PRs section and the claims about closures go beyond the template, and the Verification section cites a commit hash and CI-like outputs that a reviewer must check independently. These do not block the check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

updated the original branch to 90e80335fb7390737f09cddbddb01c6e0118ceab by merging current main (c3888da8b4). kept @only21mil's fix and regression cases intact: the canonical-parent comparison is already the smallest correct change, preserving protection for linked cleanup folders and entries. no replacement pr, rebase, force push, or label changes.

verification:

• reproduced on current main: linked-home regression fails; linked logs-folder preservation passes.
• blacksmith: all 124 cleanup tests pass; vp run check:changed verification/main passes format, lint, knip, graphql generation, server/scripts typechecks, and adjacent tests. the explicit ref pins the tested main commit inside the snapshot worker.
• real web client, system chrome, isolated symlinked home, 8-day retention: main keeps both expired files; this head removes the rotated log (11 b) and browser capture (15 b), recovers 26 b, and retains the current log. matching fixtures and settings in both captures.
• github: all 28 checks passed or skipped; mergeable. two independent read-only reviews approved this exact head without findings.

unverified this run: windows junctions, macos, native desktop/mobile. coderabbit's new automatic review was rate limited; its original review had no findings. no ui code changed.

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 90e80335fb7390737f09cddbddb01c6e0118ceab against github/main at 49a9e80e0b30e3012412bcc564bb5e52e09d1c1d on this linux boat vm. installed each checkout with vp i and ran one background dev server at a time, with an isolated home reached through a symlink. drove the web client with repo playwright and /usr/bin/google-chrome.

• created a project through the app pointing at the real repository. real codex gpt-6.1-sol turns completed on both revisions, ran git commands, and saved browser captures through the app's preview tools. generated an actual rotated trace log through normal authenticated server requests. no mocks, fixtures, replay provider, or demo data.
• set both retention rules to 8 days through settings. aged the app-generated capture and rotated log to 10 days by changing their filesystem mtimes. main's "delete now" kept both and reported "no expired files". the pr's startup sweep removed them. restored the same original bytes and mtimes to verify manual "delete now": it removed one capture (76,195 bytes) and one rotated log (10,404,702 bytes), reporting 10.00 mb recovered.
• nearby behavior: the current trace and fresh browser capture survived; a symlinked artifacts folder was skipped even with an expired capture inside; symlinked entries and their external targets survived. repeating cleanup removed nothing and reported 0 b recovered.
• blacksmith: all 124 existing cleanup tests passed. vp run check:changed verification/main passed format, lint, both knip passes, graphql generation, server/scripts typechecks, and adjacent tests. the worker's base ref was pinned to this pr's main parent c3888da8b4fa39b5edd3d0aa69479068647413cf. existing pr ci reports 28 successful checks, zero failing or pending.

short runtime transcript, after the manual cleanup:

main: browser artifacts = No expired files; rotated logs = No expired files
main: expired capture and rotated trace still exist
pr:   browser artifacts = Removed 1 browser artifacts (76,195 bytes)
pr:   rotated logs = Removed 1 rotated logs (10,404,702 bytes)
pr:   expired capture and rotated trace absent
pr:   current trace, recent capture, linked targets exist
pr:   repeat Delete now = 0 removed, 2 kept, 0 B recovered

matching data and retention rules below. settings moved from storage to maintenance on newer main; this pr changes no ui code. inspected both images and the full recording, and verified uploaded video playback and seeking in t3's browser.

Not verified

native desktop, ios/android, macos symlink aliases, windows junctions, remote/relay/tunnel connections, and other providers were not exercised on this linux vm. no performance claim: the vm reported one transient 16-second cpu-overload stall during startup/browser loading, then completed the provider and cleanup flows. no turn or app stall exceeded 10 minutes. no source changes or pushes were needed.