Fix path resolution leading slash stripping & preserve plaintext agent stream formatting - #134
Fix path resolution leading slash stripping & preserve plaintext agent stream formatting#134Spagles wants to merge 7 commits into
Conversation
…ing of empty lines
nettee
left a comment
There was a problem hiding this comment.
Found one non-blocking follow-up around regression coverage for the plaintext streaming fix. I reviewed the changed ranges on this head and the runtime change looks directionally right, but the new whitespace-preservation path still needs explicit tests so it doesn't regress quietly later.🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
| // go to stderr, which is forwarded as `stderr` events, not parsed here). | ||
| if (agent === "aider" || agent === "codewhale" || agent === "deepseek-tui") { | ||
| return [{ kind: "delta", text: trimmed.endsWith("\n") ? trimmed : trimmed + "\n" }]; | ||
| return [{ kind: "delta", text: line + "\n" }]; |
There was a problem hiding this comment.
This changes the Next-side parser to preserve raw whitespace for aider / codewhale / deepseek-tui, but the regression still isn't covered anywhere in the test suite. next/src/lib/agents/__tests__/argv.test.ts currently only exercises opencode and bob, and the PR also doesn't add a blank-line / indentation case to cli/src/__tests__/agents-invoke.test.ts even though the CLI loop changed in parallel. That matters because the old bug was exactly a line-splitting/parsing detail: a future reintroduction of trim() or if (!line) continue would silently corrupt code blocks and HTML again while the current tests stay green. Please add focused cases for leading spaces, whitespace-only lines, interior blank lines, and a trailing partial line for these plaintext agents in both the Next parser tests and the CLI invoke tests.🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.
|
Hey @Spagles — thanks for the focused fix here. @nettee's latest review is the main follow-up on this head: the runtime change looks right, but the plaintext streaming path still needs explicit regression coverage so leading spaces, blank lines, and trailing partial lines for
|
|
Hey @Spagles — the current runtime/path fix looks directionally right, and @nettee left one non-blocking follow-up on this head that’s worth tightening before the next pass. The missing piece is focused regression coverage for the plaintext streaming path: tests should explicitly cover leading spaces, whitespace-only lines, interior blank lines, and a trailing partial line for the plaintext agents in both the Next parser tests and the CLI invoke tests, so this whitespace-preservation fix can’t quietly regress later.
|
This PR fixes two issues:
findCommonPathfrom stripping leading slashes on Unix systems when computing common directory roots.aider,codewhale,deepseek-tui) so code blocks and HTML formatting aren't corrupted during streaming.Changes
Path Resolution (
cli/src/collision-resolve.ts,cli/src/index.ts,cli/src/__tests__/collision-resolve.test.ts):.filter(Boolean)onpath.sepsplits infindCommonPathso the root slash isn't lost on Unix systems.collision-resolvetest assertions to expect resolved absolute paths.Stream Parsing (
cli/src/agents-invoke.ts,next/src/lib/agents/invoke.ts,next/src/lib/agents/argv.ts):if (!line) continuecheck in the stream processing loops.Testing