fix(agent-core-v2): make large file reads resumable - #3645
Conversation
🦋 Changeset detectedLatest commit: 213aa1e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24ddebd564
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add column offsets and forward recovery for bounded long-line reads. Reject malformed UTF-16 in Read instead of replacing invalid input.
RealKai42
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3f6c86822
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Return readable text after strict UTF-16 decoding fails, with a warning retained in budgeted read continuations.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 251decf991
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 213aa1e862
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (lineOffset >= 0) return await this.readForward(readLines(), request); | ||
| const rereadsFile = detectedEncoding === undefined && (args.n_lines ?? Infinity) < -lineOffset; | ||
| const result = await this.readTail(readLines, request); | ||
| if (!result.isError && rereadsFile) { |
There was a problem hiding this comment.
When a two-pass tail request (n_lines < -line_offset) cannot fit even one complete line, finishPage returns an isError result that still contains a forward Next Read, so this guard skips the post-read stat check. If an active log is appended between the counting pass and reread while rangeEnd remains below the old EOF, the reread stops before observing the append and the continuation points at the old tail range, causing the caller to retrieve the wrong line. The fresh evidence in the final revision is the new !result.isError condition, which leaves this recovery-error path unchecked; perform the consistency check for reread results that expose a continuation as well.
Useful? React with 👍 / 👎.
Sync 147 upstream commits onto the fork. Notable upstream changes: - Remove the protocol package and relocate wire types into agent-core-v2/src/events.ts (MoonshotAI#3646) - Absorb stepRetry into the LLM requester turn state machine (already followed in the 2026-09-07 sync) - NotifyUser tool + mid-turn update panel (MoonshotAI#3524) - HEIC/HEIF/BMP image support (MoonshotAI#3649), resumable large file reads (MoonshotAI#3645), reasoning_details round-trip (MoonshotAI#3492), zstd updater manifests (MoonshotAI#3669) Conflict resolutions (all by hand, fork features preserved): - package.json: drop the empty simple-git-hooks block (upstream cleanup; not a fork feature). - docs/config-manifest.toml: take the union, then regenerate with pnpm gen:config-manifest (33 sections, fork sections intact). - test/agent/stepRetry/stepRetry.test.ts: keep upstream's slimmed retryBackoffDelays test — the fork had deleted the old fat version when stepRetry was absorbed; the util is fork-used code with no other coverage. - protocol/src/events.ts: accept upstream's package removal; the fork's compaction.started model/model_display fields already auto-merged into their new home (agent/fullCompaction/compactionOps.ts). No source still imports @moonshot-ai/protocol. Merge follow-ups: - Regenerate state/wire/config manifests; freshness tests pass. - node-sdk update-all-session-models flag test: drop the v1-engine parity half (legacy agent-core v1 is removed upstream); keep the v2 registration pin. - Fix pre-existing lint errors in merge-touched files so the lint-staged pre-commit gate passes: unused imports/variables (sdk-rpc-client-v2.ts, tui/commands/config.ts, check-import-boundaries.mjs), floating promises (sdk-rpc-client-v2.ts, fullCompaction.test.ts). - Add compactionOps.ts (model_display) to .github/FORK_OWNED_FILES so the relocated compaction-model fields are guard-listed. Verification: - pnpm run typecheck green; pnpm run lint at fork baseline (no new findings vs pre-merge HEAD). - FORK_OWNED_FILES markers all resolve. - Fork suites pass: TUI commands + session CLI (75 tests), x-opencode-session (7), update-all-session-models flag, oauth openai-compatible. - fullCompaction.test.ts: 2 failures, both pre-existing at pre-merge HEAD (baseline had 5; upstream's macOS stabilization fixed 3. EOF )
1 participant
Related Issue
No linked issue. This addresses repeated partial reads caused by overlapping file-read and tool-output limits, including long records that could not be recovered with Read alone.
Problem
Read could return up to 100 KiB, then the general 50,000-character tool-output limit would replace that result with a roughly 5,000-character preview and a spill-file path. Separate line-count and per-line limits could discard more content. Large single-line records also lacked an in-tool continuation path, which made recovery depend on access to a shell tool.
What changed
max_chars. Configure both values with[read].default_max_charsand[read].max_chars; requests above the configured maximum are capped and reported.column_offsetfor forward reads, preserve Unicode boundaries, and return exactNext Readarguments without decreasing the remaining source-line range before a line is finished. Out-of-range columns, offsets inside surrogate pairs, and columns combined with negative line offsets are rejected.n_linesis omitted or covers the tail window, retaining a character-bounded suffix. Earlier-ending ranges retain two scans and detect extra lines beyond the counted EOF or changes in file size, modification time, or inode. Single-pass reads can include appended lines observed before EOF without requiring a stable file size.Column positions refer to the displayed text of the current file, excluding line-number prefixes. This change does not introduce file snapshots, arbitrary binary/encoding support, or changes to general spill retention.
Validation
pnpm --filter @moonshot-ai/agent-core-v2 test test/os/backends/node-local/tools/read.test.ts test/agent/toolExecutor/toolExecutor.test.ts --disableConsoleIntercept --maxWorkers=4— 133 tests passed, including single-pass tail reads, append detection, same-size edits, and file replacement. Static-file tail results also matched the previous implementation in 500 differential cases.251decf:pnpm --filter @moonshot-ai/agent-core-v2 test --disableConsoleIntercept --maxWorkers=4 --exclude '**/smoke-read-pr.test.ts'— 6,424 tests across 365 files passed. The flag avoids a Vitest worker-teardown RPC error in console interception observed locally; test assertions remain enabled. An unrelated untracked local smoke-test file was excluded; all tracked tests were included. A temporary-directory cleanup race in the tower suite passed on isolated rerun.pnpm --filter @moonshot-ai/agent-core-v2 typecheck— passed.pnpm --filter @moonshot-ai/agent-core-v2 lint:imports— passed.pnpm lint— passed with warnings and no errors.pnpm -C docs run build— passed.Checklist
gen-changesetsskill and included CLI patch changesets.gen-docsguidance and updated both documentation locales withtranslate-docs; the obsolete changelog-sync script referenced by the skill is absent, so no release changelog was regenerated.