Skip to content

fix(gateway): fill bounded transcript windows across short reads#108242

Open
sunlit-deng wants to merge 1 commit into
openclaw:mainfrom
sunlit-deng:sunlit/fix/gateway-transcript-short-reads
Open

fix(gateway): fill bounded transcript windows across short reads#108242
sunlit-deng wants to merge 1 commit into
openclaw:mainfrom
sunlit-deng:sunlit/fix/gateway-transcript-short-reads

Conversation

@sunlit-deng

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where gateway transcript readers silently corrupt or drop data when positional reads return short (a documented POSIX behavior, common on network filesystems). Several bounded window reads in src/gateway/session-utils.fs.ts either ignore the returned byte count and decode the full buffer — turning the unwritten remainder into NUL garbage inside the JSONL parser — or accept a single partial read and lose the missing bytes: session title previews (readSessionTitleFieldsFromTranscript head/tail reads), session preview items (readRecentMessagesFromTranscript), the recent usage/cost snapshot (readRecentSessionUsageFromTranscript), and the recent transcript tail lines reader. Users see wrong or empty last-message previews in the session list, missing preview items, and missing cost/usage data.

Why This Change Was Made

The file adds two small local helpers, readFdRangeFully and readHandleRangeFully, that loop until the bounded window is filled or EOF (the same idiom as readLogWindowFully merged in #105066), and the seven single-shot positional reads in this file now use them and decode only bytesRead. No exported signature changes; the bounded window sizes are unchanged.

User Impact

Session list previews, session preview items, and session cost/usage snapshots stay intact when transcript reads return short, instead of showing empty or corrupted values.

Evidence

  • Regression tests (added, all fail on main): node scripts/run-vitest.mjs run src/gateway/session-utils.short-read.test.ts — main: 4 failed; this branch: 4 passed. Sibling suite session-utils.fs.test.ts: 64 passed.
  • Live proof: the real exported readers against a real transcript file, with the POSIX short-read contract applied at the fs.readSync boundary (each capped call still performs a real kernel read).

Baseline (main @ bef86c8):

$ node --import tsx live-proof.mjs
title.firstUserMessage: null
title.lastPreview     : null
preview item texts    : []
usage snapshot        : null (LOST)

After (this branch, same script and input):

$ node --import tsx live-proof.mjs
title.firstUserMessage: "first user message"
title.lastPreview     : "final preview text"
preview item texts    : ["middle reply","final preview text"]
usage snapshot        : provider=openai in=900 out=100
live-proof.mjs (save in repo root, run with `node --import tsx live-proof.mjs`)
// Drives the REAL gateway transcript readers against a real transcript file,
// with the documented POSIX short-read contract applied at the fs.readSync
// boundary (every capped call still does a real kernel read).
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
import { pathToFileURL } from "node:url";

const repoRoot = process.cwd();
const load = (rel) => import(pathToFileURL(path.resolve(repoRoot, rel)).href);
const {
  readRecentSessionUsageFromTranscript,
  readSessionPreviewItemsFromTranscript,
  readSessionTitleFieldsFromTranscript,
} = await load("src/gateway/session-utils.fs.ts");

const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-proof-gw-"));
const storePath = path.join(tmpDir, "sessions.json");
const sessionId = "proof-session";
const lines = [
  { type: "session", version: 1, id: sessionId },
  { message: { role: "user", content: "first user message" } },
  { message: { role: "assistant", content: "middle reply" } },
  {
    message: {
      role: "assistant",
      provider: "openai",
      model: "gpt-5.4",
      usage: { input: 900, output: 100, cost: { total: 0.003 } },
    },
  },
  { message: { role: "assistant", content: "final preview text" } },
];
fs.writeFileSync(
  path.join(tmpDir, `${sessionId}.jsonl`),
  lines.map((l) => JSON.stringify(l)).join("\n"),
  "utf-8",
);

// POSIX short-read contract at the read boundary: at most 16 bytes per call.
const realReadSync = fs.readSync;
fs.readSync = (fd, buffer, offset, length, position) =>
  realReadSync(fd, buffer, offset, Math.min(length, 16), position);

const title = readSessionTitleFieldsFromTranscript(sessionId, storePath);
const items = readSessionPreviewItemsFromTranscript(
  sessionId,
  storePath,
  undefined,
  undefined,
  3,
  120,
);
const usage = readRecentSessionUsageFromTranscript(sessionId, storePath, undefined, undefined, 64 * 1024);
fs.readSync = realReadSync;
fs.rmSync(tmpDir, { recursive: true, force: true });

console.log(`title.firstUserMessage: ${JSON.stringify(title.firstUserMessage)}`);
console.log(`title.lastPreview     : ${JSON.stringify(title.lastMessagePreview)}`);
console.log(`preview item texts    : ${JSON.stringify(items.map((i) => i.text))}`);
console.log(`usage snapshot        : ${usage ? `provider=${usage.modelProvider} in=${usage.inputTokens} out=${usage.outputTokens}` : "null (LOST)"}`);
process.exit(0);

AI-assisted: built with Codex

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: M labels Jul 15, 2026
@sunlit-deng
sunlit-deng force-pushed the sunlit/fix/gateway-transcript-short-reads branch 2 times, most recently from d1af51a to fdfb2d8 Compare July 16, 2026 03:01
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 25, 2026, 1:24 PM ET / 17:24 UTC.

ClawSweeper review

What this changes

This PR completes bounded gateway transcript reads after positive short reads and adds regression coverage for session titles, previews, and usage snapshots.

Merge readiness

⚠️ Needs maintainer review before merge - 3 items remain

The supplied PR evidence describes a focused short-read regression fix with real terminal proof and green checks, but the read-only sandbox failed before I could inspect the exact current-main code, complete PR diff, scoped policies, maintainer notes, or git history. Keep this PR open for a source-backed maintainer review rather than making a cleanup or merge decision from context alone.

Priority: P2
Reviewed head: e7b0e99ab75c3ea746f8842a32ac7ce8403f76f2

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The submitted runtime proof is strong, but independent source, current-main, and history inspection was blocked by the sandbox.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body provides after-fix terminal output from the exported readers against a real temporary transcript file with short reads injected at the filesystem boundary; no private data is shown.
Patch quality 🦐 gold shrimp (3/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body provides after-fix terminal output from the exported readers against a real temporary transcript file with short reads injected at the filesystem boundary; no private data is shown.
Evidence reviewed 4 items Submitted live behavior proof: The PR body shows a real temporary transcript file read through the exported gateway readers while the fs.readSync boundary caps each kernel read to 16 bytes; its reported after-fix output restores title, preview-item, and usage values.
Focused regression coverage: The supplied pull-file data adds src/gateway/session-utils.short-read.test.ts and reports four short-read regressions that fail on the stated baseline and pass on this branch.
Related precedent: The context links merged PR #105066, which established the same bounded-read completion invariant for log tails; this supports the requested behavior but does not establish current gateway ownership or current-main inclusion.
Findings None None.
Security None None.

How this fits together

Gateway session utilities read bounded JSONL transcript windows to produce session titles, recent-message previews, and usage data. Those summaries feed session-list and diagnostic-facing gateway behavior after the transcript bytes are parsed.

flowchart LR
  A[Session transcript JSONL] --> B[Bounded positional read]
  B --> C[Complete positive short reads]
  C --> D[Parse transcript window]
  D --> E[Session title and preview]
  D --> F[Usage snapshot]
Loading

Before merge

  • Resolve merge risk (P1) - The branch is behind its recorded base, and the mandatory current-main three-way comparison could not be performed in the failed read-only sandbox.
  • Resolve merge risk (P1) - The PR description claims broader reader changes than the supplied current pull-file summary shows; inspect the complete exact-head diff before merging to confirm the intended coverage remains present.
  • Complete next step (P2) - The next action is a human source-backed review after sandbox recovery, not an automated repair: the available context is promising but insufficient for the required current-main and history verdict.
Agent review details

Security

None.

PR surface

Source +3, Tests +145. Total +148 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 3 0 +3
Tests 1 145 0 +145
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 148 0 +148

Review metrics

None.

Stored data model

Persistent data-model change detected: serialized state: src/gateway/session-utils.short-read.test.ts, unknown-data-model-change: src/gateway/session-utils.short-read.test.ts. Confirm migration or upgrade compatibility proof before merge.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Review the exact head against current main, confirm every bounded gateway transcript-read path uses the established completion helper and that the new regressions still distinguish the branch from main, then merge only if the focused behavior remains unique.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Review the exact head against current main, confirm every bounded gateway transcript-read path uses the established completion helper and that the new regressions still distinguish the branch from main, then merge only if the focused behavior remains unique.

Do we have a high-confidence way to reproduce the issue?

Unclear: the PR provides a concrete real-file short-read reproduction and claimed baseline failure, but this review environment could not inspect or execute the current-main reader path.

Is this the best way to solve the issue?

Unclear: completing positive short reads is the appropriate narrow pattern indicated by the related merged repairs, but the exact gateway callers and current-main implementation still need direct comparison.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 1b2dc1819012.

Labels

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P2: This is a focused gateway correctness repair for session summary data, with no supplied evidence of broad runtime unavailability or security impact.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides after-fix terminal output from the exported readers against a real temporary transcript file with short reads injected at the filesystem boundary; no private data is shown.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix terminal output from the exported readers against a real temporary transcript file with short reads injected at the filesystem boundary; no private data is shown.

Evidence

What I checked:

  • Submitted live behavior proof: The PR body shows a real temporary transcript file read through the exported gateway readers while the fs.readSync boundary caps each kernel read to 16 bytes; its reported after-fix output restores title, preview-item, and usage values. (src/gateway/session-utils.fs.ts:1749, e7b0e99ab75c)
  • Focused regression coverage: The supplied pull-file data adds src/gateway/session-utils.short-read.test.ts and reports four short-read regressions that fail on the stated baseline and pass on this branch. (src/gateway/session-utils.short-read.test.ts:1, e7b0e99ab75c)
  • Related precedent: The context links merged PR fix(logs): preserve log tails across short reads #105066, which established the same bounded-read completion invariant for log tails; this supports the requested behavior but does not establish current gateway ownership or current-main inclusion. (src/logging/log-tail.ts, c94da0df6f38)
  • Inspection environment failure: The sandbox rejected even pwd before execution with bwrap: loopback: Failed RTM_NEWADDR: Operation not permitted, preventing the mandatory local source, policy, history, and maintainer-note passes.

Likely related people:

  • qingminglong: The supplied context identifies this person as author of the merged log-tail short-read repair that the PR explicitly follows; current gateway-file history could not be inspected. (role: adjacent short-read invariant contributor; confidence: low; commits: c94da0df6f38; files: src/logging/log-tail.ts, src/logging/log-tail.test.ts)
  • masatohoshino: The supplied context identifies this person as author of the merged Anthropic transcript reverse-scan short-read repair; it is an adjacent implementation, not proof of gateway ownership. (role: adjacent transcript-reader contributor; confidence: low; commits: f1205f5f0bcd; files: extensions/anthropic/session-catalog.ts, extensions/anthropic/session-catalog.test.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Refresh the branch against current main and complete an exact-head source review of the bounded reader call sites before merge.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)
  • reviewed 2026-07-16T03:13:45.103Z sha fdfb2d8 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-16T14:11:09.088Z sha 34ef500 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-16T23:57:39.607Z sha 24d1316 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-17T14:34:03.703Z sha 00f7a76 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-17T15:48:29.038Z sha a64048c :: needs maintainer review before merge. :: none

@sunlit-deng
sunlit-deng force-pushed the sunlit/fix/gateway-transcript-short-reads branch from fdfb2d8 to 34ef500 Compare July 16, 2026 14:05
@sunlit-deng
sunlit-deng force-pushed the sunlit/fix/gateway-transcript-short-reads branch from 34ef500 to 24d1316 Compare July 16, 2026 23:40
@sunlit-deng
sunlit-deng force-pushed the sunlit/fix/gateway-transcript-short-reads branch 3 times, most recently from a64048c to 293f3c8 Compare July 22, 2026 23:33
@sunlit-deng
sunlit-deng force-pushed the sunlit/fix/gateway-transcript-short-reads branch from 293f3c8 to e7b0e99 Compare July 24, 2026 03:05
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant