Skip to content

fix(server): bound thread activity payload hydration - #8991

Closed
rcawston wants to merge 1 commit into
pingdotgg:mainfrom
rcawston:fix/bound-thread-activity-payloads
Closed

rcawston wants to merge 1 commit into
pingdotgg:mainfrom
rcawston:fix/bound-thread-activity-payloads

Conversation

@rcawston

@rcawston rcawston commented Sep 1, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

  • Limit thread-detail activity hydration to the 500 most recent activities.
  • Cap individual hydrated activity payloads at 512 KiB and the aggregate payload budget at 8 MiB.
  • Preserve unresolved approval and user-input requests even when they fall outside the recent activity window.
  • Add focused coverage for row limits, aggregate limits, oversized-payload markers, and pinned requests.

Why

Severity: high (regular-use blocker).

Thread detail reads selected and JSON-decoded every persisted activity payload. Tool-heavy conversations can accumulate hundreds of megabytes or more of command output, so opening or refreshing an otherwise normal long-running conversation could exhaust Node's V8 heap. The backend would terminate and restart, disconnecting the client and dropping the in-flight conversation update.

The read model only needs a bounded recent timeline plus unresolved requests. Applying limits before payload decoding keeps thread detail usable without deleting durable history or hiding actionable requests.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes
  • I included a video for animation/interaction changes

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 1, 2026
activity.kind,
activity.summary,
activity.payload_json AS "payload",
CASE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Layers/ProjectionSnapshotQuery.ts:1372

Oversized pinned approval.requested and user-input.requested activities are returned without requestId (and without questions for user input), so the client cannot derive an actionable request and the provider turn remains blocked. The truncation branch replaces the entire payload with a generic marker; keep pinned unresolved-request payloads intact (or truncate while preserving the required request fields).

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts around line 1372:

Oversized pinned `approval.requested` and `user-input.requested` activities are returned without `requestId` (and without `questions` for user input), so the client cannot derive an actionable request and the provider turn remains blocked. The truncation branch replaces the entire payload with a generic marker; keep pinned unresolved-request payloads intact (or truncate while preserving the required request fields).

ORDER BY sequence DESC, created_at DESC, activity_id DESC
) AS recent_order,
MIN(
length(payload_json),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Layers/ProjectionSnapshotQuery.ts:1046

length(payload_json) counts Unicode code points for SQLite TEXT, not UTF-8 bytes, so multi-byte JSON payloads bypass the 512 KiB per-payload cutoff and the 8 MiB cumulative budget; this can hydrate tens of MiB. Use a byte-length expression such as length(CAST(payload_json AS BLOB)) consistently for the cutoff, truncation marker, and cumulative budget.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProjectionSnapshotQuery.ts around line 1046:

`length(payload_json)` counts Unicode code points for SQLite `TEXT`, not UTF-8 bytes, so multi-byte JSON payloads bypass the 512 KiB per-payload cutoff and the 8 MiB cumulative budget; this can hydrate tens of MiB. Use a byte-length expression such as `length(CAST(payload_json AS BLOB))` consistently for the cutoff, truncation marker, and cumulative budget.

@macroscopeapp

macroscopeapp Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This production change alters thread-detail payloads and selectively omits persisted activities to enforce hydration limits across full and windowed reads. Unresolved high-severity concerns remain around preserving actionable request fields and correctly enforcing byte limits, so the runtime behavior requires human review.

Not approved because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@t3dotgg

t3dotgg commented Sep 4, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6 Astra (preview) responding on behalf of Theo

This was closed as part of an automated cleanup pass. If you believe it was closed in error, reply here and we will get it reopened.

Closing this older-query implementation in favor of merged #9000, which projects client payloads in 25-row batches. This patch misses that client path and truncates fields required by pending requests. Its oversized-payload fixture remains available at f25c1ae, but it needs adaptation to the current query. This does not mean every large-payload memory failure is fixed.

@t3dotgg t3dotgg closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants