[Bug]: Desktop log pruning deletes noncanonical numeric-looking neighbors that the shared log sink keeps

Đang mở Phù hợp với người mới
#35,353 0 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
78/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
node.js, typescript
Lĩnh vực
backend, desktop

Hướng nghiên cứu

Đọc apps/desktop/src/app/DesktopObservability.ts, đặc biệt là pruneOverflowBackups và makeRotatingLogFileWriter, sau đó so sánh cách xử lý hậu tố với việc dọn dẹp khi khởi động trong packages/shared/src/logging.ts. Dùng trường hợp tái hiện được mô tả trong issue hoặc thêm một bài kiểm thử tập trung trong apps/desktop/src/app; công việc hoàn tất khi chỉ các bản sao lưu tràn số dạng chuẩn bị xóa, các tệp lân cận không theo dạng chuẩn vẫn còn và hành vi xoay vòng hiện có vẫn hoạt động.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

Before submitting
  • I searched existing issues and did not find a duplicate.
  • I included enough detail to reproduce or investigate the problem.
Area

apps/desktop

Steps to reproduce

Use a checkout of the commit under Version or commit with its repository dependencies installed. Save the script below as repro.sh and run sh repro.sh from the checkout root. It creates its fixtures under a mktemp -d directory with a private HOME, TMPDIR and T3CODE_HOME, installs one temporary test at apps/desktop/src/app/zz-verify-prune.test.ts (refusing to overwrite an existing file), runs it with env -u NODE_ENV vp test run, and removes the test and fixtures on exit. It does not launch Electron.

The test seeds three directories with a current log containing existing, in-range backups .1 and .2, canonical overflow backups .11 and .50, and hand-made neighbors .011, .11.0, .0xb, .+11 and .notes. It then calls the exported production makeRotatingLogFileWriter with the backend caller's options (server-child.log, default maxFiles 10) and with the renderer-history caller's options (renderer-history.ndjson, maxBytes 256 KiB, maxFiles 2), appends one line through each writer, and as a control writes one line through the shared RotatingFileSink with maxFiles 10. The script exits 0 only when the desktop writer deletes the four noncanonical neighbors, the shared sink keeps them, both delete .11 and .50, both keep .1 and .2 with their contents, and both append correctly, so its assertions pin the current defect.

Reproduction script: repro.sh
#!/bin/sh
set -eu
checkout=$(cd "${1:-.}" && pwd)
fixture=$(mktemp -d)
test_file="$checkout/apps/desktop/src/app/zz-verify-prune.test.ts"
test ! -e "$test_file"
trap 'rm -f "$test_file"; rm -rf "$fixture"' EXIT HUP INT TERM
mkdir -p "$fixture/fake-home" "$fixture/tmp" "$fixture/t3"
export HOME="$fixture/fake-home" TMPDIR="$fixture/tmp" T3CODE_HOME="$fixture/t3" PRUNE_FIXTURE="$fixture"
export PATH="$checkout/node_modules/.bin:$PATH"
cat > "$test_file" <<'TEST'
import * as NodeServices from "@effect/platform-node/NodeServices";
import { assert, it } from "@effect/vitest";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import { RotatingFileSink } from "../../../../packages/shared/src/logging.ts";
import { makeRotatingLogFileWriter } from "./DesktopObservability.ts";

it.effect("verifies desktop pruning versus the shared sink", () => Effect.gen(function* () {
  const fs = yield* FileSystem.FileSystem;
  const root = process.env.PRUNE_FIXTURE!;
  const canonical = ["11", "50"];
  const foreign = ["011", "11.0", "0xb", "+11", "notes"];
  const all = ["1", "2", ...canonical, ...foreign];
  for (const [kind, base, maxFiles] of [
    ["desktop-default", "server-child.log", undefined],
    ["desktop-renderer", "renderer-history.ndjson", 2],
    ["shared-control", "server-child.log", 10],
  ] as const) {
    const dir = `${root}/${kind}`;
    const filePath = `${dir}/${base}`;
    yield* fs.makeDirectory(dir);
    yield* fs.writeFileString(filePath, "existing\n");
    for (const suffix of all) yield* fs.writeFileString(`${filePath}.${suffix}`, `marker:${suffix}`);
    if (kind === "shared-control") {
      new RotatingFileSink({ filePath, maxBytes: 1024, maxFiles: 10 }).write("appended\n");
    } else {
      const writer = yield* makeRotatingLogFileWriter({ filePath, ...(maxFiles === undefined ? {} : { maxBytes: 256 * 1024, maxFiles }) });
      yield* writer.writeText("appended\n");
    }
    const names = yield* fs.readDirectory(dir);
    const deletedCanonical = canonical.filter(s => !names.includes(`${base}.${s}`));
    const deletedForeign = foreign.filter(s => !names.includes(`${base}.${s}`));
    const retainedInRange = ["1", "2"].every(s => names.includes(`${base}.${s}`));
    const appendCorrect = (yield* fs.readFileString(filePath)) === "existing\nappended\n";
    process.stdout.write(JSON.stringify({kind, deletedCanonical, deletedForeign, retainedInRange, appendCorrect}) + "\n");
    assert.deepEqual(deletedCanonical, canonical);
    assert.deepEqual(deletedForeign, kind === "shared-control" ? [] : ["011", "11.0", "0xb", "+11"]);
    assert.isTrue(retainedInRange);
    assert.isTrue(appendCorrect);
    for (const suffix of all.filter(s => names.includes(`${base}.${s}`))) {
      assert.strictEqual(yield* fs.readFileString(`${filePath}.${suffix}`), `marker:${suffix}`);
    }
  }
  process.stdout.write("DEFECT CONFIRMED; shared control correct; in-range backups and append correct\n");
}).pipe(Effect.provide(NodeServices.layer)));
TEST
cd "$checkout"
env -u NODE_ENV vp test run apps/desktop/src/app/zz-verify-prune.test.ts
Expected behavior

The desktop writer should delete only the backup names it owns above maxFiles and leave other files beside its log alone. Its own rotation only ever produces plain integer suffixes, through withSuffix and the rotation loop. The shared RotatingFileSink startup prune already deletes a suffix only when String(suffix) equals the suffix text, and its tests are named "startup prunes only canonical rotation suffixes". No existing desktop test pins deleting the noncanonical names as intended.

Actual behavior

Summary. The desktop app's rotating log writer, makeRotatingLogFileWriter in apps/desktop/src/app/DesktopObservability.ts, deletes files next to its log whose suffix only parses to a number above maxFiles, such as server-child.log.011, server-child.log.11.0, server-child.log.0xb and server-child.log.+11. Its own rotation never creates those names, and the shared RotatingFileSink in packages/shared/src/logging.ts keeps them. Canonical overflow backups, in-range backups and appends behave correctly in both writers. Only hand-made files are established as triggers: no T3 Code naming scheme, other tool or crash-dump producer of these names is known, and no user incident is known, so this is a cleanup-consistency bug rather than an established data-loss path for ordinary desktop use.

In the reproduction, the desktop writer deletes .011, .11.0, .0xb and .+11 when it is created, with both the backend options and the renderer-history options. The shared sink keeps those four files. Both writers delete the canonical overflow backups .11 and .50, keep .1, .2 and .notes with their contents unchanged, and append appended after existing in the current log.

The desktop pruneOverflowBackups converts each suffix with Number and removes any integer above maxFiles without checking that the text is the canonical integer spelling, so Number("011"), Number("11.0"), Number("0xb") and Number("+11") all become 11. The writer runs this prune at creation (line 323) before returning. Two production callers use it: the backend output writer for server-child.log or server-child-<instance>.log with the default maxFiles of 10, and the renderer-history writer with maxFiles 2. The prune therefore runs when each of these writers is created, which includes backend instance logger creation; this report does not claim it runs only once per app start.

A repository search for the two base names and for crash-dump naming found no writer of these neighboring names, and the desktop writer's history back to its origin in the shared sink also used plain integer suffixes; that history check is bounded and does not cover every past release or outside tool. A user who manually saved a copy such as server-child.log.011 in the log folder would lose it the next time the writer is created.

Evidence
  • Expected source: the integer-only withSuffix rotation in apps/desktop/src/app/DesktopObservability.ts lines 258-285, and the canonical-suffix check in the shared sink's pruneOverflowBackups in packages/shared/src/logging.ts lines 147-157, exercised as the passing shared-sink control in the reproduction.
  • Failure source: the reproduction output under Logs or stack traces, and the Number-only suffix check in apps/desktop/src/app/DesktopObservability.ts lines 262-269.
  • Evidence provenance: observed
  • Local verification: reproduced
  • Reproduction completeness: complete

The script calls the exported production writer with the production callers' option shapes against the real Node filesystem services. It does not start Electron, so an integrated desktop launch was not observed. An independent verification run passed the same script at the tested commit, and the script above was rerun from a clean checkout of that commit with the same result. The same focused script also passed on upstream main at 86ae1747ad62e301307bcf7a7ce7a6f1fcae3d16 using the tested commit's installed dependency tree, not a fresh install; that run printed unrelated web gt-react import-resolution warnings while the desktop test passed. Between the tested commit and that main revision, the only change to these files adds an exit-time flush to tracer setup and leaves the writer and the renderer-history call unchanged.

Related

Merged #27503 added the canonical-suffix check to the shared synchronous and asynchronous sinks only; it does not mention the desktop writer or record a decision to exclude it, has no linked bug issue, and reports no known user incident. Closed #24755 (unbounded boot-service.log growth) and closed #4543 (Windows first-launch failure whose trace mentions this writer) concern different failures. Hypothesis, not established: the desktop copy kept the broader Number-based prune after the shared sinks gained the canonical-suffix guard.

Restoration check

With the script's input, creating the desktop writer with either production option shape keeps .011, .11.0, .0xb and .+11 unchanged, matching the shared-sink control. As controls, it still deletes the canonical overflow backups .11 and .50, keeps .1, .2 and .notes with their contents, and appends correctly. The script's deletedForeign assertion for the desktop cases encodes the current defect and needs updating to an empty list for this check.

Impact

Minor bug or occasional failure

Version or commit

T3 Code 0.1.9 at d9805b2ca667be7caec8c9035950564ab96704c3, rerun from a clean checkout; the same script also passed on upstream main at 86ae1747ad62e301307bcf7a7ce7a6f1fcae3d16 using the tested commit's installed dependency tree (not a fresh install).

Environment

macOS on arm64, Node v26.12.1, the repository's Vitest 5.0.5 runner, production desktop writer and shared sink against the real filesystem; no Electron launch.

Logs or stack traces

The script's output, keeping only its result lines and test summary:

{"kind":"desktop-default","deletedCanonical":["11","50"],"deletedForeign":["011","11.0","0xb","+11"],"retainedInRange":true,"appendCorrect":true}
{"kind":"desktop-renderer","deletedCanonical":["11","50"],"deletedForeign":["011","11.0","0xb","+11"],"retainedInRange":true,"appendCorrect":true}
{"kind":"shared-control","deletedCanonical":["11","50"],"deletedForeign":[],"retainedInRange":true,"appendCorrect":true}
DEFECT CONFIRMED; shared control correct; in-range backups and append correct
Test Files  1 passed (1)
Tests  1 passed (1)
Screenshots, recordings, or supporting files

No response

Workaround

Keep manual copies of a log outside the log folder or away from its basename prefix, or give them a nonnumeric suffix such as .notes; the reproduction shows .notes surviving in the same directory. No automatic producer of the affected filenames is known.

Ngôn ngữ chính
TypeScript
Star
24.8k
Fork
6.4k
Merge trung bình
4 giờ 16 phút
Pull request đã merge (30 ngày)
352

Chuẩn bị môi trường

Mở trong Codespaces

Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của pingdotgg/t3code

Tất cả issue của pingdotgg/t3code

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.