Skip to content

fix(server): retain bounded runtime diagnostic details - #10087

Open
juliusmarminge wants to merge 3 commits into
mainfrom
fix/retain-runtime-diagnostic-tail
Open

juliusmarminge wants to merge 3 commits into
mainfrom
fix/retain-runtime-diagnostic-tail

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 5, 2026 •

Copy link
Copy Markdown
Member

Runtime diagnostics can lose their line and column when the durable work-log message is shortened to 180 characters.

Keep that compact message and the existing warning summary. For longer runtime error/warning messages, retain plain-text detail up to 2,048 JavaScript string code units including the truncation marker. Preserve an existing warning detail, banners and session errors unchanged.

Evidence

  • The report's 233-character message and a fictional counterpart fail four receipt-driven SQLite readback assertions on main; all pass with this change.
  • All 92 ingestion/projection tests and the strengthened 500-row transport case passed, covering boundaries, Unicode, explicit warning details and unchanged compact fields. Server types and targeted lint passed.
  • An inert adapter sends a 500-row window through real production HTTP/gzip and WebSocket/deflate transports. Both client production functions consume the actual resulting activities, including mobile expanded/copy text.
  • Independent root runs pass all 92 ingestion/projection tests, the strengthened transport case and 12 both-client checks.
  • The matching actual-client captures below replay the exact fictional before/after ingestion records through the unchanged diagnostic renderer. Same viewport, expanded row and scroll position. No native WSL2/provider collection is claimed.

Before:

Before: the retained diagnostic ends before its line and column

After:

After: the expanded diagnostic retains line 2 column 50

Human decision required

The new retention limit increases transfer size. With 500 varied cap-sized diagnostics, the snapshot grows from 244,276 to 1,274,276 JSON bytes, or 73,616 to 784,783 gzip bytes. Highly escaped text reaches about 7 MB decoded JSON. Confirm the 2,048-code-unit tradeoff before merging.

Related to #1084, which stays open. Text beyond the cap can still lose its tail. Explicit warning details keep their existing precedence and are not newly capped, so this is not a universal payload limit. This does not revive the banner changes proposed by ecatuogno1 in #1202.

Prepared by GPT 6 Astra via Codex in T3 Code.

Note

Retain bounded runtime diagnostic details in runtimeEventToActivities

  • Long runtime error and warning messages now keep a compact message field capped at 180 characters plus a derived detail field capped at 2,048 characters when no explicit detail is supplied.
  • Explicit warning detail values are preserved unchanged; short runtime messages do not gain a derived detail.
  • Other event branches (e.g. tool-denied) are unchanged.
  • Risk: consumers that previously saw only the truncated compact message now receive an additional detail field on long runtime error/warning activities — check any activity readers in apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts and downstream transports for unexpected payload growth.

Macroscope summarized e67db5c.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 5, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This focused fix improves diagnostic visibility while adding up to 2,048 characters to every long runtime error or warning persisted and delivered to clients. At high event counts, that creates material SQLite, HTTP, and WebSocket payload growth, so the retention and transfer tradeoff warrants human confirmation.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 13.5 KiB −28 B (−0.2%) 15.1 KiB ✅
Codex Thread snapshot wire 7.0 KiB 7.0 KiB −3 B (−0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −25 B (−0.4%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 57.1 KiB 57.0 KiB −88 B (−0.2%) 66.4 KiB ✅
Codex Live turn messages 10 8 −2 (−20.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.6 KiB +20 B (+0.1%) 15.1 KiB ✅
Claude Thread snapshot wire 7.0 KiB 7.0 KiB +2 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.5 KiB 6.5 KiB +18 B (+0.3%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.8 KiB 57.9 KiB +88 B (+0.1%) 66.4 KiB ✅
Claude Live turn messages 8 10 +2 (+25.0%) 21 ✅

Baseline: 2271a27 · PR result: e67db5c · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.8 KiB
  • Claude decoded thread snapshot: 114.5 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant