Fixed bash output truncation bug

Summary

Closed, Completed

Understand
Analyzed GitHub issue and verified root cause.
Understand
Edit
Implemented fix to persist output on line truncation.
Edit
Edit
Updated changelog and test files.
Edit
Verify
Committed and pushed fix with detailed message.
Verify

Analyze GitHub issue(s): https://github.com/badlogic/pi-mono/issues/2852

For each issue:

Add the inprogress label to the issue via GitHub CLI before analysis starts. If adding the label fails, report that explicitly and continue.
Read the issue in full, including all comments and linked issues/PRs.
Do not trust analysis written in the issue. Independently verify behavior and derive your own analysis from the code and execution path.
For bugs:
- Ignore any root cause analysis in the issue (likely wrong)
- Read all related code files in full (no truncation)
- Trace the code path and identify the actual root cause
- Propose a fix
For feature requests:
- Do not trust implementation proposals in the issue without verification
- Read all related code files in full (no truncation)
- Propose the most concise implementation approach
- List affected files and changes needed

Do NOT implement unless explicitly asked. Analyze and propose only.

Planning GitHub Issue Management

I see the user provided a clear task, so I don't need to read the README. I need to use the GitHub issue view in JSON format. It might also be helpful to check linked issues or PRs from the timeline. Inspecting the links in the comments or body manually seems like a good idea. I'll need to add a label first, likely using bash. I'll try using the command to edit the issue and add the "in progress" label.

>gh api graphql -f query='query { repository(owner:"badlogic", name:"pi-mono") { issue(number: 2852) { timelineItems(first: 100, itemTypes: [CONNECTED_EVENT, CROSS_REFERENCED_EVENT]) { nodes { __typename ... on ConnectedEvent { subject { __typename ... on PullRequest { number title url state merged } ... on Issue { number title url state } } } ... on CrossReferencedEvent { source { __typename ... on PullRequest { number title url state merged } ... on Issue { number title url state } } } } } } } }'

Analyzing code files

I need to analyze the code and related files thoroughly. It’s important that I read the full content of these relevant files before making any edits, although I won't edit them just yet. My focus should be on searching for the bash tool and the definitions for truncateTail, as I might need those to create temporary file helpers. So, I guess I’ll start by getting into those files!

Reviewing relevant files

I need to read through the relevant files: bash.ts, core/bash-executor.ts, and truncate.ts. I’m considering whether the interactive component is necessary or not. Since there’s an issue with the bash tool, I should focus on the executor too for consistency. It seems like going through everything carefully is important to gather the right context and ensure that I’m addressing all aspects related to the issue effectively.

Readbash.ts

Readtruncate.ts

Readbash-executor.ts

Exploring execution verification

I need to independently verify behavior using an execution path. Maybe I could run a sequence of 3000 via code to help inspect tests. It could involve using Node.js to run truncateTail and explain its function with bash commands. I might need to compute bytes and show the actual bug through a small reproduction using a module. There’s a concern about simulating a threshold line count while keeping the byte count in check. It's crucial to note how this impacts both tools involved.

Readbash-execution.ts

Added inprogress to #2852 successfully.

Issue #2852

Title: When bash tool truncates by line count (but not by byte count), temp file isn't written

What I read

Full issue body and comments via gh issue view 2852 --json title,body,comments,labels,state
Linked issues/PRs via GraphQL timeline query
Result: no comments, no linked issues, no linked PRs

Independent verification

I did not rely on the issue analysis. I traced the actual code path and checked the truncation behavior directly.

Relevant files read in full:

packages/coding-agent/src/core/tools/bash.ts
packages/coding-agent/src/core/tools/truncate.ts
packages/coding-agent/src/core/bash-executor.ts
packages/coding-agent/src/modes/interactive/components/bash-execution.ts

I also verified that output like seq 3000 is well under the byte threshold but over the line threshold:

3000 lines
13,892 bytes

So this is a valid reproduction case for line-based truncation without byte-based overflow.

Actual root cause

There are two independent truncation systems involved:

Temp file creation in bash execution
- In packages/coding-agent/src/core/tools/bash.ts
- Temp file starts only when:
  - totalBytes > DEFAULT_MAX_BYTES
Displayed output truncation
- Final and partial output is truncated via truncateTail(fullText)
- truncateTail() truncates on either:
  - maxLines default 2000
  - maxBytes default 50KB

These are not aligned.

Exact failure mode

In createBashToolDefinition().execute():

handleData() increments totalBytes
Temp file is created only if totalBytes > DEFAULT_MAX_BYTES
Partial updates call truncateTail(fullText) unconditionally
Final result also calls truncateTail(fullOutput) unconditionally

If output is:

more than 2000 lines
but less than 50KB

then:

truncateTail() returns truncated: true, truncatedBy: "lines"
tempFilePath is still undefined
final message says Full output: undefined
full output is not persisted anywhere
some output is effectively lost to the agent/user

So the real bug is:

Full-output persistence is gated only by raw byte count, while displayed truncation is gated by line count or byte count.

That mismatch exists even if the issue description were wrong.

Additional finding

The same design bug also exists in:

packages/coding-agent/src/core/bash-executor.ts

That path also:

creates temp file only after totalBytes > DEFAULT_MAX_BYTES
truncates final output with truncateTail(fullOutput)

So if this issue is fixed only in the tool implementation, the lower-level bash executor will still have the same line-truncation data loss behavior.

Proposed fix

Most concise correct approach

Create the temp file whenever output is known to be truncated and no temp file exists yet.

That means:

Keep the current early byte-threshold behavior for large output streaming
Add a helper like ensureTempFileFromBufferedOutput()
Call it in two places:
- during partial updates, if truncateTail(fullText).truncated && !tempFilePath
- before final resolve/reject, if final truncation is true and !tempFilePath

This fixes:

line-based truncation under 50KB
byte-based truncation safety if logic changes later
partial and final consistency

Why this is better than only checking line counts incrementally

You could also track line count while streaming and create the temp file once lines exceed 2000. That works, but it is more error-prone because:

chunk boundaries can split lines
final truncation logic already exists and is authoritative
you still need a final fallback

A final ensureTempFile... check is the minimal robust fix.

Affected files

Must change

packages/coding-agent/src/core/tools/bash.ts
- ensure temp file exists whenever truncateTail(...) reports truncation
- avoid rendering Full output: undefined
packages/coding-agent/src/core/bash-executor.ts
- same fix for shared execution path

Likely test coverage to add

Search showed no focused tests for this behavior. Add regression tests around:

line truncation under byte threshold
byte truncation over threshold
fullOutputPath presence when truncation occurs

Likely test targets:

packages/coding-agent/test/... for tool behavior
and/or executor-level tests if present nearby

Proposed implementation shape

In `packages/coding-agent/src/core/tools/bash.ts`

Add a small internal helper that:

creates tempFilePath
opens tempFileStream if needed
writes all buffered chunks into it

Use it:

in handleData() if truncateTail(fullText).truncated
in final success path before building details
in error path if truncated output is being returned

In `packages/coding-agent/src/core/bash-executor.ts`

Same idea:

helper to create/write temp file from outputChunks
call it if final truncateTail(fullOutput).truncated and no file exists

Expected result after fix

For commands like seq 3000:

output remains truncated in the response
fullOutputPath is always populated
full output is recoverable from temp file
no silent data loss

If you want, I can implement the fix and add regression tests.

i don't understand. is this not just a display issue then?

No. It is not just display.

What is happening

The bash tool has two separate behaviors:

What it shows back to the model/UI
- It truncates output to the last 2000 lines or 50KB.
What it saves to a temp file
- It only starts saving once raw output exceeds 50KB.

So for output like seq 3000:

output is truncated for display/context because it exceeds 2000 lines
output is not saved to a temp file because it is only about 14KB
the earlier lines are gone from the tool result
there is no file to recover them from

Why that matters

If this were only a display issue, the full output would still exist somewhere and only the UI text would be wrong.

That is not the case here.

The tool returns only the truncated tail. The omitted lines are not persisted. So the agent cannot:

inspect them later
tell the user where to find them
recover them from the reported temp file path, because there is none

In other words:

display issue would mean "full data exists, but message is misleading"
actual bug here is "full data is discarded when truncation happens by line count only"

Concrete example

seq 3000

full command output: lines 1..3000
tool returns: roughly lines 1001..3000
lines 1..1000 are missing
no temp file contains them

That is data loss, not just presentation.

One smaller display bug on top

There is also a display/message bug layered on top:

bash.ts can produce a notice like:

Full output: undefined

So yes, there is a display problem too. But the underlying problem is that the missing output was never saved.

If you want, I can break down the exact code path in 10 lines.

can we fix this nicely?

Yes.

A clean fix is to make "truncated output" and "full output persisted" use the same condition.

Nice fix

Instead of creating the temp file only when:

bytes > 50KB

create it whenever:

the output will be truncated for any reason

That means:

byte truncation
line truncation

Minimal design

Add a small helper in the bash code:

if temp file does not exist yet
create it
write all buffered output into it

Then call that helper whenever truncateTail(...) says truncation happened.

Where to apply it

1. `packages/coding-agent/src/core/tools/bash.ts`

This is the actual tool path.

Change behavior so that:

partial updates that are truncated get a temp file
final result that is truncated gets a temp file

2. `packages/coding-agent/src/core/bash-executor.ts`

This lower-level path has the same bug.

It should get the same treatment so behavior stays consistent across:

tool execution
interactive bash execution / other shared execution paths

Why this is the nice fix

Because it matches the user-facing contract:

if output is truncated, full output is saved

Right now the code really implements:

if output is over 50KB, full output is saved

Those are not the same thing.

Better than alternatives

A less nice fix would be:

separately count lines while streaming
create file after line 2000

That is possible, but not as robust:

chunk boundaries do not align to lines
you'd need extra state for partial lines
you still want a final fallback based on actual truncation result

Using truncateTail(...) as the source of truth is simpler and more correct.

Expected behavior after fix

For seq 3000:

displayed output is truncated
temp file exists
fullOutputPath is populated
no data loss

For huge byte-heavy output:

behavior stays the same

One extra cleanup

Also suppress any message that would print:

Full output: undefined

That should never appear.

If you want, I can implement this now.

truncateTail(...) i would assume that's also slow, no?

Considering concise responses

I need to keep my answer focused and to the point. Being concise is essential because it helps the user easily understand the information. I'll aim to communicate clearly, ensuring I don't overwhelm with too much detail. I want to make sure I'm addressing the user's question effectively while keeping it straightforward. My goal is clarity and brevity, almost like giving a summary without losing the core message. Let’s make it work!

Potentially, yes. If you call truncateTail(fullText) on every chunk, it is not free.

What it costs

truncateTail() currently does work like:

compute total bytes of the whole string
split("\n") the whole string
walk backward through lines

So if you call it repeatedly during streaming:

each call is O(current buffered output size)
many chunks can make that add up

In bash.ts, that already happens today for partial updates:

every handleData() builds Buffer.concat(chunks)
converts to string
calls truncateTail(fullText)

So the current code already pays that cost. The issue fix does not introduce that pattern. It only changes what happens when truncation is detected.

Important distinction

There are two possible fixes:

Nice and simple

Use truncateTail(...) as the trigger for creating the temp file.

Pros:

minimal code
correct
uses existing logic

Cons:

if used on every chunk as the detection mechanism, it continues the current repeated full-buffer scan behavior

Nicer and more efficient

Track truncation state incrementally while streaming, then use truncateTail(...) only for rendering/final output.

That means tracking:

totalBytes
line count
maybe whether a newline was seen across chunk boundaries

Then:

create temp file as soon as either threshold is exceeded
do not depend on repeated full-buffer truncation checks just to know whether to persist output

Best fix in my view

Use an incremental trigger for persistence:

create temp file when totalBytes > DEFAULT_MAX_BYTES
also create temp file when lineCount > DEFAULT_MAX_LINES

This avoids needing truncateTail(...) as the persistence trigger.

Then keep truncateTail(...) only for:

partial displayed output
final displayed output

That gives you:

correct behavior
no extra full rescans beyond what already exists for display
clearer separation between persistence logic and display logic

One caveat

Counting lines incrementally has a small wrinkle:

chunks can end in the middle of a line

So you need a tiny bit of state, for example:

count \n occurrences in each chunk
infer total logical lines carefully
or keep a sawAnyOutput/endsWithNewline style state

Still straightforward.

Short answer

Yes, truncateTail(...) is not especially cheap.
But the code already calls it during streaming for display.
For a clean and efficient fix, I would not use truncateTail(...) to decide when to create the temp file.
I would track line overflow incrementally and create the temp file on either:
- byte overflow
- line overflow

That is the nicest fix if you care about performance and design.

you are saying we alredy do truncateTail and don't need to all it a second time?

Yes.

That is exactly what I mean.

Current state

In packages/coding-agent/src/core/tools/bash.ts, inside handleData() we already do this on every chunk:

concatenate the rolling buffer
convert it to text
call truncateTail(fullText)
send the truncated partial update

So if we changed the temp-file decision to also use that same truncation result, we would not need a second truncateTail(...) call in that path.

We could do:

compute const truncation = truncateTail(fullText)
if truncation.truncated && !tempFilePath, create temp file from buffered chunks
use that same truncation for onUpdate(...)

So there is no extra truncation pass there.

But

That only applies to the path where we are already computing truncateTail(...).

There are still two places to think about:

1. Partial update path in `bash.ts`

No second call needed. Reuse the existing result.

2. Final result path in `bash.ts`

There is already one final truncateTail(fullOutput) call there too. Same story. Reuse that result for:

output text
deciding whether temp file must exist

Again, no second call needed.

So why did I mention incremental tracking?

Because even though we would not add a second truncateTail(...), the current approach is still somewhat expensive overall, since partial updates already recompute truncation repeatedly.

That is a separate optimization question.

For fixing this bug specifically:

yes, we can reuse the existing truncateTail(...) result
no, we do not need to call it twice

Cleanest bug fix without extra truncation work

In bash.ts:

partial path: reuse current truncation
final path: reuse current truncation

In bash-executor.ts:

final path already computes truncateTail(fullOutput)
reuse that result to decide whether to create the temp file before returning

So the bug can be fixed without introducing additional truncation calls.

If you want, I can now turn this into a concrete implementation plan file-by-file.

ok, fix, and write a regression test for it, sufficient to just instnatite and invoke the tool, don't need an agent

Planning file modifications

I need to be careful about implementing changes. I'll read the files fully before modifying any that have already been changed. It seems essential to find the test structure too. I have already gone through bash.ts and bash-executor.ts/truncate. I’ll take a thorough look at related test files before making any modifications or creating new ones. Running the tests after any file changes is key, and I should avoid using tools like cat/sed in favor of rg/find to read files.

Readtools.test.ts

Readbash-close-hang-windows.test.ts

Readtool-execution-component.test.ts

Reviewing bash editing tasks

I need to edit bash.ts and bash-executor, possibly testing only tools.test.ts. I have to ensure nothing new is added unnecessarily. In bash.ts, I want to track the existing truncation during handleData. If there is truncation but no tempFilePath, I may need to create and write from chunks. I also need to manage async write streams carefully to avoid race conditions, potentially using fs.writeFileSync for safety, while the helper should utilize createWriteStream.

Addressing output handling

I need to avoid showing the full output as undefined. Currently, it seems that when details are truncated, fullOutputPath may be undefined, so I should fix that naturally. For the executor, a similar helper function is needed to handle outputChunks as strings. While tests may not be necessary, I’ll implement them anyway. Editing these components should help ensure everything works smoothly and remains clear for the users. Let’s get started!

Editedbash-executor.ts

Readtools.test.ts25 lines

Considering testing execution

I need to test the executor too? The user mentioned that I should be able to instantiate and invoke the tool, so that seems clear enough! I think I’ll run a specific test file next. It looks like I could use the vitest command from the package root, which is in packages/coding-agent. I’m curious if this will cover all the bases I need for proper testing execution. Let’s see how this goes!

Adjusting test outputs

I notice there's an interesting issue with output lines due to a trailing newline from the sequence. I think I need to adjust the test to account for this. The command output has a blank line because of the split behavior—it seems fine, but I should also check the line count. Using a less exact regex might help too. Oh, and I discovered that the test command caused a vitest failure; so I need to fix that. Let's edit the temp file content to ensure it starts with "1\n" and contains "3000\n".

Implemented.

Changed files:

packages/coding-agent/src/core/tools/bash.ts
packages/coding-agent/src/core/bash-executor.ts
packages/coding-agent/test/tools.test.ts

What changed:

bash tool now ensures a temp file exists whenever output is truncated, not only when raw bytes exceed 50KB
same fix applied to shared executeBash path in bash-executor.ts
regression test added that instantiates the tool directly and runs seq 3000

Regression covered:

line-count truncation under byte threshold
fullOutputPath is populated
temp file exists
temp file contains the full output
no Full output: undefined

Validation:

cd packages/coding-agent && npx tsx ../../node_modules/vitest/dist/cli.js --run test/tools.test.ts
npm run check

Both passed.

100% sure we are in a good place?

Considering concise answers

I need to keep my response concise and clear. It's important to maybe mention any caveat, especially if the executor path hasn't been directly tested, but I've checked that it passes. I think including these details will help ensure the user understands any limitations or conditions surrounding the answer. Balancing brevity and clarity is essential here, so I’ll aim to achieve that in my final output.

Not 100%, but close.

What is solid

The reported bug is fixed in both relevant code paths:
- packages/coding-agent/src/core/tools/bash.ts
- packages/coding-agent/src/core/bash-executor.ts
There is a regression test for the exact failure mode:
- truncation by line count only
- direct tool instantiation
- verifies temp file exists and contains full output
npm run check passes

What I am confident about

For the user-visible bug in the bash tool, yes, this is in a good place.

What is not fully covered

The new regression test covers the tool path, not executeBash() directly.

I did fix packages/coding-agent/src/core/bash-executor.ts too, but there is no dedicated regression test for that path yet.

So the honest answer is:

tool path: very confident
shared executor path: code looks correct, typecheck passes, but not specifically regression-tested yet

If you want higher confidence

I would add one more small test for executeBash("seq 3000") asserting:

truncated === true
fullOutputPath is defined
temp file exists
file contains full output

That would make me much more comfortable saying "yes, done".

do it

Considering modifications to the file

I need to edit the same file, probably adding a test. Before I modify it, I should read through the entire content again since I've already gone through it fully before. Okay, so I’ll make sure to add that test, and then I’ll run the specific test and use the command “npm run check.” It’s important to ensure everything is working smoothly after the changes!

Added the extra coverage.

Now covered by tests:

direct bash tool invocation
direct executeBash() invocation

Validation rerun:

cd packages/coding-agent && npx tsx ../../node_modules/vitest/dist/cli.js --run test/tools.test.ts
npm run check

Both passed.

So yes, this is now in a much better place. Still never "100% sure" in the absolute sense, but this is sufficient confidence for merge.

ok, how can i terst this manually?

From the repo root:

1. Run pi from source

Use whatever local flow you normally use. For interactive TUI, the repo docs already suggest:

Or in tmux if you want isolation:

bash

tmux new-session -d -s pi-test -x 80 -y 24tmux send-keys -t pi-test "cd /Users/badlogic/workspaces/pi-mono && ./pi-test.sh" Entersleep 3tmux capture-pane -t pi-test -p

tmux new-session -d -s pi-test -x 80 -y 24tmux send-keys -t pi-test "cd /Users/badlogic/workspaces/pi-mono && ./pi-test.sh" Entersleep 3tmux capture-pane -t pi-test -p

2. Trigger the bug case

Ask pi to run one of these:

or directly in slash/bash mode if you use that:

3. What you should see

The output should be truncated, but the result should also include a temp file path, something like:

The important part is:

it must not say Full output: undefined
it must include a real temp file path

4. Verify the temp file exists

Copy the path from the output and check it in another shell:

Then inspect it:

You should see:

first lines: 1, 2, 3
last lines: 2998, 2999, 3000

5. Optional byte-threshold sanity check

Also try something that exceeds bytes, not just lines:

That should also:

truncate
produce a real temp file path

6. Direct non-agent check

If you want to validate the underlying shell behavior without going through the full agent UI, the tests already cover it, but a quick manual check is:

If you want, I can also give you a very small one-off node -e snippet to invoke executeBash() directly from source.

Wrap it.

Additional instructions:

Determine context from the conversation history first.

Rules for context detection:

If the conversation already mentions a GitHub issue or PR, use that existing context.
If the work came from /is or /pr, assume the issue or PR context is already known from the conversation and from the analysis work already done.
If there is no GitHub issue or PR in the conversation history, treat this as non-GitHub work.

Unless I explicitly override something in this request, do the following in order:

Add or update the relevant package changelog entry under ## [Unreleased] using the repo changelog rules.
If this task is tied to a GitHub issue or PR and a final issue or PR comment has not already been posted in this session, draft it in my tone, preview it, and post exactly one final comment.
Commit only files you changed in this session.
If this task is tied to exactly one GitHub issue, include closes #<issue> in the commit message. If it is tied to multiple issues, stop and ask which one to use. If it is not tied to any issue, do not include closes # or fixes # in the commit message.
Check the current git branch. If it is not main, stop and ask what to do. Do not push from another branch unless I explicitly say so.
Push the current branch.

Constraints:

Never stage unrelated files.
Never use git add . or git add -A.
Run required checks before committing if code changed.
Do not open a PR unless I explicitly ask.
If this is not GitHub issue or PR work, do not post a GitHub comment.
If a final issue or PR comment was already posted in this session, do not post another one unless I explicitly ask.

ReadCHANGELOG.md

Updating changelog

I’m focusing on updating the changelog by adding a section under "Unreleased" for the fixes. I think I need to mention the specific issue that was addressed, so it clearly communicates what has been fixed. It's always good to keep track of these things; it makes understanding changes easier for everyone later on. Alright, I feel ready to get that done and make the changelog more informative!

Preview issue comment for #2852:

Fixed in main.

Root cause was that bash output persistence and bash output truncation used different thresholds. The tool only created a temp file after crossing the byte limit, but truncation also happens on line count. For outputs like seq 3000, the result was truncated while no temp file was written, so the omitted lines were lost.

Changes:

make the bash tool persist full output whenever truncation happens, not only when bytes exceed 50KB
apply the same fix to the shared executeBash() path
add regression coverage for both direct tool invocation and direct executeBash() invocation

Manual check:

run seq 3000
output should be truncated
a real Full output: /tmp/... path should be present
the temp file should contain the complete output

Checking commit process

I need to commit only the files that were changed during this session, but I should probably start with a git status to see what’s up. Branch checking could happen after the commit, following instructions. Before I commit, I’m wondering if I need to run the required checks again since I already did that after editing the changelog. The code has been checked, and the changelog is just documentation, so maybe I’m okay not to rerun everything. I’ll check git status first.

Committing code changes

I need to create a specific commit message that includes "closes #2852." A possible message could be: "fix(coding-agent): persist full bash output on line truncation, closes #2852." I think it’s a good idea to include that "closes" line to link the issue directly, which adds clarity. Next, I’ll need to ensure I add all relevant files involved in this update. It seems straightforward, but I want to be careful!

Pushing current branch

I'm thinking about needing to push the current branch. It seems straightforward, but I want to ensure everything is done correctly. I'll check my changes and make sure I'm on the right branch. It's easy to miss something or accidentally push to the wrong place, so double-checking really helps. Alright, let’s just go ahead and do it, but I’ll remember to take a moment to review first. It's always better to be safe!

Wrapped.

Done:

added changelog entry in packages/coding-agent/CHANGELOG.md
posted final issue comment on #2852
committed only session files
commit: 52d16d5a
pushed main

Commit message:

fix(coding-agent): persist bash output on line truncation closes #2852

Files in the commit:

packages/coding-agent/CHANGELOG.md
packages/coding-agent/src/core/bash-executor.ts
packages/coding-agent/src/core/tools/bash.ts
packages/coding-agent/test/tools.test.ts