Skip to content

flaky test: shell-run-manager secret redaction across a soft wrap #2978

Closed

Description

@Astro-Han

redacts a secret across a soft wrap and the scrollback/screen boundary in packages/runtime/src/__tests__/shell-run-manager.test.ts fails intermittently on CI.

Observed: run 31716295489, test_workspaces. The case failed at 27.6ms; the enclosing ShellRunProcessManager suite took 8.1s. It surfaced on #2962, which touches only packages/runtime-host/src/client and packages/eval and does not change packages/runtime at all. main was green at the time. Running the case alone locally passes in 549ms.

Suspected cause: the assertion depends on when the PTY flushes across the soft-wrap and scrollback/screen boundary, which shifts under CI load. The fix is to wait for a settled terminal state rather than a particular flush ordering.

This is not a redaction defect — the production behaviour under test looks correct; only the test's timing assumption is unsound.

Activity

  1. Astro-Han commented on Aug 13, 2026

    @Astro-Han
    ContributorAuthor

    Second occurrence, and it points somewhere other than where this issue currently points.

    run 31729090882, test_workspaces, on #2971. That commit adds one JSON file under packages/eval/experiments/ and touches nothing in packages/runtime; the previous commit on the same branch was green.

    The failure this time is not a timing assertion:

    ✖ redacts a secret across a soft wrap and the scrollback/screen boundary (99.368469ms)
      Error: file is not a database
        at OperationalStateDatabaseOwner.transaction (packages/storage/dist/operational-state-store.js:132:23)
        at SqliteShellRunStore.createShellRun (packages/storage/dist/shell-run-store.js:18:21)
        at ShellRunProcessManager.createStartingRecord (packages/runtime/dist/shell-run-manager.js:723:33)
        at ShellRunProcessManager.startPty (packages/runtime/dist/shell-run-manager.js:612:41)
      code: 'ERR_SQLITE_ERROR', errcode: 26, errstr: 'file is not a database'
    

    The test never reaches its assertions — it dies opening the operational state store, before the PTY produces any output. So the soft-wrap flush ordering in the description is not the cause here, and possibly was not the cause in the first occurrence either; a store that fails to open would surface as whatever the enclosing case happened to assert next.

    SQLite error 26 means the header bytes are not a database. That is a file-level problem, not a race on the data: a truncated or zero-length file, a path collision with a non-database file, or two runs sharing a temporary directory that one of them created before the other finished writing the header.

    Worth checking whether the store path is derived per test case or per suite, and whether anything under the temporary root is reused across concurrent workspace lanes. If the path is shared, the first occurrence and this one have the same cause and the timing hypothesis can be dropped.

    I am not picking this up as part of #2971 — noting it so the next person starts from the store rather than from the terminal.

  2. M4n5ter commented on Aug 14, 2026

    @M4n5ter
    Member

    Third occurrence with the same SQLite failure signature:

    The only failing case was again:

    redacts a secret across a soft wrap and the scrollback/screen boundary
    

    It failed before reaching the redaction assertions:

    Error: file is not a database
      at OperationalStateDatabaseOwner.transaction
      at SqliteShellRunStore.createShellRun
      at ShellRunProcessManager.createStartingRecord
      at ShellRunProcessManager.startPty
    code: 'ERR_SQLITE_ERROR', errcode: 26, errstr: 'file is not a database'
    

    This PR does not modify shell-run-manager, shell-run-store, or operational-state-store. Its local full Runtime suite passed, and the subsequent main run 31774040205 passed test_workspaces. This reinforces that the failure is in temporary operational-state database setup/lifecycle rather than the PTY redaction behavior or PR #2993.

    The failed jobs were rerun without any code changes. Attempt 2 failed in the same test with the same ERR_SQLITE_ERROR / error 26 signature: test_workspaces job 94686518079. This occurrence is therefore reproducible across two fresh CI jobs for the same SHA, despite the corresponding main workspace lane passing.

  3. M4n5ter commented on Aug 14, 2026

    @M4n5ter
    Member
    English

    After investigating the repeated failures, this does not appear to be a secret-redaction flake or a temporary SQLite path collision. The evidence points to an upstream Unix write-lifecycle defect in node-pty.

    The preceding test intentionally generates more than 1 MiB of terminal protocol replies while the child is not reading stdin. In node-pty@1.2.0-beta.14, CustomWriteStream submits writes asynchronously using fs.write() against the raw PTY file descriptor. When the PTY exits, its cleanup does not safely drain or cancel writes that are already in flight.

    This creates an fd-reuse race:

    1. The PTY fd is closed while an asynchronous write is still pending.
    2. The OS may reuse that fd for the SQLite database opened by the next test.
    3. If the stale write runs before reuse, node-pty logs EBADF.
    4. If it runs after reuse, terminal reply bytes are written into the SQLite file, and its first transaction fails with ERR_SQLITE_ERROR: file is not a database.

    This matches all three CI failures:

    • The protocol-reply pressure test completes immediately before the failure.
    • The following redaction test fails while creating its initial database record, before its own PTY starts.
    • Successful runs log Unhandled pty write error ... EBADF between these two tests; failing runs do not log EBADF and instead observe SQLite corruption.
    • The database root and command workspace are independently created with mkdtemp(), while the workspace runner also assigns an isolated TMPDIR, so a shared-path collision does not explain the failures.

    I also reproduced the race independently with node-pty@1.2.0-beta.14: out of 100 iterations, 97 ended with the stale write reporting EBADF, while 3 corrupted a newly opened SQLite database with the same file is not a database error.

    node-pty@1.2.0-beta.15 does not fix this. Its release contains a Windows ConPTY fix and maintenance changes, and the complete beta.14...beta.15 diff does not modify the Unix write implementation.

    Relevant failing jobs:

    Therefore, I suggest treating this issue as an upstream node-pty raw-fd write-after-close/fd-reuse defect. We can isolate or relocate the pressure test as a temporary CI mitigation, but adding retries, sleeps, or changing the redaction assertion would only hide the symptom and should not be considered a root-cause fix.

    中文

    经过对多次失败的深入调查,这并不是 secret redaction 测试自身的偶发问题,也不是 SQLite 临时路径发生了碰撞。现有证据指向 node-pty 在 Unix 平台上的上游写入生命周期缺陷。

    前一个测试会在子进程不读取 stdin 的情况下,故意生成超过 1 MiB 的终端协议回复。在 node-pty@1.2.0-beta.14 中,CustomWriteStream 使用原始 PTY 文件描述符,通过 fs.write() 异步提交写入。PTY 退出时,其清理逻辑不会安全地排空或取消已经在途的写入。

    因此会产生文件描述符复用竞态:

    1. PTY 文件描述符关闭时,仍有异步写入尚未完成。
    2. 操作系统可能将同一个 fd 分配给下一项测试刚打开的 SQLite 数据库。
    3. 如果残留写入发生在 fd 复用前,node-pty 会打印 EBADF。
    4. 如果残留写入发生在 fd 复用后,终端协议回复就会被写入 SQLite 文件,导致首次事务报 ERR_SQLITE_ERROR: file is not a database。

    这与三次 CI 失败的表现完全吻合:

    • 协议回复压力测试总是在失败前刚刚完成。
    • 紧随其后的 redaction 测试在创建第一条数据库记录时失败,此时它自己的 PTY 尚未启动。
    • 成功运行会在这两个测试之间打印 Unhandled pty write error ... EBADF;失败运行没有打印 EBADF,而是出现 SQLite 损坏。
    • 数据库根目录和命令工作目录分别通过 mkdtemp() 独立创建,workspace runner 也分配了隔离的 TMPDIR,因此共享路径碰撞无法解释这些失败。

    我还使用 node-pty@1.2.0-beta.14 独立复现了这个竞态:100 次运行中,97 次由残留写入报出 EBADF,另外 3 次损坏了随后打开的 SQLite 数据库,并产生相同的 file is not a database 错误。

    node-pty@1.2.0-beta.15 没有解决这个问题。其发布内容只有 Windows ConPTY 修复和维护性变更;完整的 beta.14...beta.15 diff没有修改 Unix 写入实现。

    相关失败任务:

    因此,建议将本 issue 重新定性为上游 node-pty 的 raw-fd close 后残留写入/fd 复用缺陷。我们可以隔离或移动该压力测试,作为临时的 CI 缓解措施;但增加重试、延时或修改 redaction 断言都只是在隐藏症状,不应被视为根因修复。

  4. self-assigned this
    on Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workinghelp wantedExtra attention is needed

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions