Skip to content

Mirror local run logs that contain non-UTF-8 bytes (OR-581) - #746

Closed
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1792663021-or581-non-utf8-local-logs
Closed

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1792663021-or581-non-utf8-local-logs

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Fixes OR-581.

Summary

  • On Windows, native tools such as MATLAB and cmd.exe write output in the console code page, not UTF-8. localbox::stream_logs read local-runs/<id>/log with read_to_string and treated the decode error like a missing file, returning Ok(skip) on every poll. As a result, the supervisor (tail_logs_local) never mirrored a line, and run-logs/<id>.log stayed at 0 bytes even after the run finished.
  • The fix reads the raw bytes and decodes them with String::from_utf8_lossy. Invalid bytes become U+FFFD, and the line count stays the same, so the skip/seen bookkeeping still works. A missing file still returns Ok(skip). stream_logs has only one caller, tail_logs_local, and it needed no change.
  • Adds the stream_logs_keeps_lines_that_are_not_utf8 test.
  • For the reviewer: decoding is lossy, so a non-UTF-8 byte appears as U+FFFD in the mirrored log (for example, 25°C becomes 25\uFFFDC). Output is no longer dropped, but the characters are not decoded through the console code page.

Test plan

  • CI is green, including windows build, test. Scope review (fresh context): yes. Greptile: 5/5, no notes.
  • Windows run on main: Claude Code ran train.sh from the dashboard as a local run. The script prints 2,000 lines, then a cmd.exe line containing °. The run finished done/exit 0. The run-dir log is 44,949 bytes, but orx logs reports a 0-byte run-logs file.
  • Same flow on this branch (a2aa2de): the run finished done/exit 0, and orx logs reports a 44,950-byte run-logs file whose preview ends with the summary line and DONE exit 0.

Bug reproduction on main (Windows)

Bug reproduction on main

Post-fix check on this branch (Windows)

Post-fix functionality on Windows

Automation improvements

  • scripts/dev-slot.mjs exits with "requires macOS or Linux" on Windows, so the Windows sessions start an isolated orx up by hand. Adding Windows support, or documenting the ORX_DATA_DIR/ORX_CACHE_DIR/XDG_CONFIG_HOME setup, would save setup time in every Windows run.
  • CODEX_AUTH_JSON is stale (codex exec fails with refresh_token_reused). Refreshing it from a new ChatGPT Codex login would make Codex usable for issues that need it.

Link to Devin session: https://app.devin.ai/sessions/53f7e0b94ad343b4b9abfb76f9372abe
Open in Devin Desktop: https://app.devin.ai/desktop/session/53f7e0b94ad343b4b9abfb76f9372abe?variant=devin

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

Reads local run logs as bytes and replaces invalid UTF-8 instead of skipping the whole file.

  • Local run logs keep lines that contain invalid UTF-8 bytes.

Reviews (1) · Last reviewed commit: "Mirror local run logs that contain non-U..." · Reviewed by Greptile

@sox8502 sox8502 added the awaiting review Devin bugfix PR is open and needs human review before merge label Oct 22, 2026
@sox8502 sox8502 self-assigned this Oct 22, 2026
@paperclover

Copy link
Copy Markdown
Contributor

comment written by Claude Opus 5.5

superseded by #862.

@paperclover paperclover closed this Nov 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review Devin bugfix PR is open and needs human review before merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants