Repository navigation
fix(cli): flush session exports before exit - #57347
Dreamer-HIT wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. I ran the #57266 reproduction with each checkout's CLI from source. On the base, piping session export through | cat or a slow reader gave 262,144 of 1,206,978 bytes (invalid JSON, exit 0). With this PR, both give the full, valid JSON. import-export.test.ts passes, the new test fails on the base as expected, and the CLI typecheck passes.
Public API: unchanged.
Two points, both inline:
- EPIPE:
session export <id> | headnow exits 1 and printsEPIPE: broken pipe, write. Before, it exited 0 with no message. - Test size: the regression test is large for a fix this small.
| catch: (cause) => cause, | ||
| }) | ||
| process.stdout.write(JSON.stringify(data, null, 2) + EOL) | ||
| yield* Effect.tryPromise({ |
There was a problem hiding this comment.
When the reader closes early (opencode session export <id> | head, or | jq exiting), this rejects with EPIPE. The handler's catch then prints EPIPE: broken pipe, write and sets exit code 1. I saw this here; on the base the same pipe exits 0 silently. A closed reader isn't really an export failure, so I'd ignore EPIPE here, as #52955 does for api get with its writeStdout() helper. Reusing that helper once it lands would keep the two commands consistent.
| } | ||
| }, 15_000) | ||
|
|
||
| test("export flushes a large transcript before exiting", async () => { |
There was a problem hiding this comment.
The fix is about 6 lines, but this test is 60+: it writes a reader script, quotes the shell pipeline separately for sh and cmd, and needs windowsVerbatimArguments. It does catch the bug (it fails on the base with 262,144 bytes), but it's a lot to maintain for this one change. I'd drop it to keep the change minimal, or cut it down to the POSIX sh -c pipe and compare only the byte count.
Issue for this PR
Closes #57266
Type of change
What does this PR do?
Wait for the stdout write callback before
session exportreturns and the CLI exits, preventing silently truncated JSON in pipes.Add a real shell-pipe regression that checks the byte count and SHA-256 of a large Unicode transcript. Ordinary Bun subprocess capture masked the failure. This complements #52955 (API output) and #55893 (Console output).
How did you verify your code works?
Bun 1.4.2:
bun run checkpassed (34 typechecks). Locally materialized the checkout's symlinked declaration file for checking, then restored it.mktempand retained-image link failures reproduce on the base; two ACP cleanupEBUSYfailures pass in isolation before and after. A repeated full-file WSL run hit the existing raw-export test's 15s timeout; the focused regression passes.Screenshots / recordings
N/A — CLI output.
Checklist
Added by the opencode bot (AI-generated).
Related: #52925, #55525