Conversation
…read + pipe-through-head context-cmd Backports the same two perf fixes that landed on gemini.ts in PR #898 — the bugs are identical because grok.ts and gemini.ts share the same readHead + runContextCmd helper shape (sibling-port pattern from slice-15 / slice-16). P1 — Read only file head instead of loading full file: Original used readFileSync + Buffer.subarray which loads the entire file into memory then slices. For large artifacts (logs, dumps) this is a memory-spike regression vs bash 'head -c 20000'. Replaced with openSync + readSync(fd, buf, 0, bytes, 0) which reads only the first FILE_HEAD_BYTES bytes from disk. Wrapped in try/finally for fd cleanup. P2 — Truncate context command output at source: Original used spawnSync with maxBuffer up to 64MB then sliced to CTX_HEAD_BYTES afterward. For high-volume commands (wide 'git diff' ranges) this blocks longer and uses more memory than the bash original's '... | head -c 20000' pipeline that short-circuits at the boundary. Reframed wrapped command as: (cmd) 2>&1 | head -c <N> so the shell pipeline truncates at source. Same trust contract preserved (user supplies cmd; we just augment with head-c). Same fixes verified on gemini.ts in #898 (100MB test file reads only 20000 bytes via the new head-only path).
![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: 6b138c01df
ℹ️ 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.
Pull request overview
Backports the same perf-oriented behavior from gemini.ts to tools/peer-call/grok.ts to avoid unnecessary memory usage and long blocking when collecting file/context command output for peer-call prompts.
Changes:
- Update
readHeadto read only the firstFILE_HEAD_BYTESfrom disk viaopenSync/readSync(instead ofreadFileSync+ slicing). - Update
runContextCmdto truncate context command output at the source by piping throughhead -c CTX_HEAD_BYTES.
Both reviewers caught: runContextCmd was returning only result.stdout, which drops diagnostics emitted on the shell's OWN stderr (parse/syntax errors in --context-cmd). The (cmd) 2>&1 redirect only captures the command's stdout/stderr; the SHELL's stderr (where parse errors land) falls outside that redirect. Fix: concat result.stdout + result.stderr, then slice to CTX_HEAD_BYTES. On a clean run stderr is empty so output is unchanged; on malformed cmd the parse error reaches the prompt instead of an empty context block. Same fix needed on gemini.ts in #898 — opening a follow-up commit there.
AceHack
added a commit
that referenced
this pull request
Apr 30, 2026
…igration (#898) * ts(B-0086): port 1 peer-call sibling (.sh→.ts) — slice 16 of TS/Bun migration * ts(slice-16, wip 1/N): port peer-call/gemini (.sh→.ts) Sibling of slice-15 grok port (PR #896). Wraps 'gemini -p' non-interactive headless mode. Read-only safety preserved (--approval-mode plan + --skip-trust per the Copilot fix on the bash original PR #28). Reuses the structural pattern from grok.ts: - classifyFlag + MutableArgState parser (under cog-complexity 15) - spawnSync + stdio:inherit for live LLM streaming - isRegularFile + readHead helpers - /bin/sh -c for context-cmd (same trust as bash eval) - PREAMBLE preserved verbatim with Gemini-specific framing (proposes role per four-ferry consensus) Lint-clean: bun --bun tsc --noEmit + eslint strictTypeChecked + sonarjs all pass. Argument-validation byte-equivalent. Note: the same CodeQL js/indirect-command-line-injection alert will fire here as on grok.ts; per B-0107 (filed in PR #897) the per-PR dismissal pattern applies. Will dismiss after PR opens. * review(slice-16): address Codex P1 + P2 — head-only file read + pipe-through-head context-cmd Two real Codex findings on PR #898 (both also apply to grok.ts — backport in follow-up PR): P1 — Read only file head instead of loading full file: Original used readFileSync + Buffer.subarray which loads the entire file into memory then slices. For large artifacts (logs, dumps) this is a memory-spike regression vs bash 'head -c 20000'. Replaced with openSync + readSync(fd, buf, 0, bytes, 0) which reads only the first FILE_HEAD_BYTES bytes from disk. Wrapped in try/finally for fd cleanup. P2 — Truncate context command output at source: Original used spawnSync with maxBuffer up to 64MB then sliced to CTX_HEAD_BYTES afterward. For high-volume commands (wide 'git diff' ranges) this blocks longer and uses more memory than the bash original's '... | head -c 20000' pipeline that short-circuits at the boundary. Reframed wrapped command as: (cmd) 2>&1 | head -c <N> so the shell pipeline truncates at source. Same trust contract preserved (user supplies cmd; we just augment with head-c). Verified locally: 100MB test file reads only 20000 bytes via the new head-only path. * review(slice-16): preserve shell parse errors per Codex P2 + Copilot (#899 finding backport) Same fix as #899 backport — runContextCmd was returning only result.stdout, dropping shell-process stderr (where parse errors land, OUTSIDE the (cmd) 2>&1 redirect). Concat stdout+stderr then slice to CTX_HEAD_BYTES. * review(slice-16): address #898 P1+P2 — exit codes, spawn classification, bash shell, file-read errors Five real findings from Copilot+Codex on PR #898: Copilot P1 — Exit codes 0|2|64 per repo-scripting.md: Switched return 1 → return 64 for invocation/usage errors (--model/--file/--context-cmd missing values, unknown flag, empty prompt). Aligns with the conventions in docs/best- practices/repo-scripting.md §exit-codes. Process-related errors (gemini missing on PATH, build-prompt error) keep 1 for tooling/input failure. Copilot P1 — spawnSync result.status ?? 1 collapses launch failures: Added classifySpawnFailure helper (4-case: status set / ENOENT → 127 / signal / other) — same pattern from PR #887. Distinguishes ENOENT/permission/signal from a normal non-zero exit, with a contextual stderr message. Codex P2 — eval vs /bin/sh syntax difference: Switched /bin/sh -c → /bin/bash -c so the bash original's eval semantics (accepting bash-only features like `[[ ]]`, brace expansion, process substitution) are preserved. /bin/sh on Ubuntu is dash and rejects these. Codex P2 — file-read failures silently dropping context: readHead() now returns ReadHeadResult { ok, content, error }. buildFullPrompt() propagates the error to the user via stderr instead of pretending context was attached when read failed (permission denied, race, etc.). Threads on already-addressed findings (PRRT_kwDOSF9kNM5-pd16 runContextCmd full-buffering + PRRT_kwDOSF9kNM5-pd2a readHead full-file-read) were resolved in the parse-error-fix commit ceed1e7. Threads on PRRT_kwDOSF9kNM5-pd2U PREAMBLE-names: pushing back because the PREAMBLE is prompt content sent verbatim to the LLM peer, not pure code-surface; changing it diverges from the bash original byte-equivalence. A separate task to retool PREAMBLE attribution on both bash AND TS together is the right shape, not a one-side rename here. * review(slice-16) round-2: revert exit-code 64 → 1; fix commandAvailable Three new findings on the latest push: Codex P2 (PRRT_kwDOSF9kNM5-pon2) + Copilot P0 x2 — exit codes: Last commit switched to exit 64 per docs/best-practices/repo- scripting.md. Reviewers correctly point out tools/peer-call/ README.md is the more specific spec and says 0/1/2 uniform across all three peer-call wrappers. Specific wins on overlap. Reverted ArgError.exitCode 64 → 1 + the empty-prompt return 64 → 1. Matches grok.ts (slice 15) and the bash original. Copilot P1 — commandAvailable() shape: Was using `<cmd> --version` and requiring exit 0. Some CLIs exit non-zero on --version. Bash uses `command -v <cmd>` (PATH existence check, no execution). Switched to `spawnSync('/bin/sh', ['-c', \`command -v "${cmd}"\`])` to match bash semantics. Copilot P1 (PRRT_kwDOSF9kNM5-ppuR) — PREAMBLE names: same as prior thread; pushing back with same rationale (PREAMBLE is verbatim prompt content sent to LLM, not pure code-surface; preserving bash-equivalence). Will reply on the new thread the same way. Copilot P2 (PRRT_kwDOSF9kNM5-ppt6) — PR description says /bin/sh but code uses /bin/bash. The /bin/bash switch was a Codex P2 from the prior round (preserve eval semantics for bash-only features). Will update PR description, not the code.
4 tasks
AceHack
added a commit
that referenced
this pull request
Apr 30, 2026
…sure (#903) Per the consolidated-row-pattern (rows from 03:41Z + 05:01Z arc closures): when a session lands many small commits across multiple ticks, a single consolidated row summarizing the arc is more signal-dense than N minimal rows. Covers the slices 15-19 arc that landed after the 05:43Z slice-14 row: - #896 slice 15 (grok.ts) — peer-call cluster opens - #898 slice 16 (gemini.ts) — peer-call sibling - #899 backport to grok from #898 review-cycle findings - #900 slice 17 (codex.ts) — peer-call cluster closes - #897 B-0107 row — CodeQL dismissal pattern - #901 slice 18 (daily-cost-report.ts) — budget wrapper - #902 slice 19 (project-runway.ts) — budget cluster closes (in flight) Three new substrate observations recorded for future-Otto: - sibling-port-cost decreases monotonically (round-2/3 fixes bake into later siblings proactively) - kernel-pipe vs JS-space stream interleaving distinction (bash `2>&1` merges shell-side; `result.stdout + result.stderr` in JS does not preserve chronological ordering) - fix-the-bug + file-the-row + implement-the-row + closeout pattern is the durable shape (B-0106 + B-0107 are both worked examples) Cron 98fc7424 still armed. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backports the same two perf fixes that landed on
gemini.tsin #898 togrok.ts. The bugs are identical because grok.ts and gemini.ts share the samereadHead+runContextCmdhelper shape (sibling-port pattern from slice-15 / slice-16).P1 — Read only file head instead of loading full file
Original used
readFileSync+Buffer.subarraywhich loads the entire file into memory then slices. For large artifacts (logs, dumps) this is a memory-spike regression vs bashhead -c 20000. Replaced withopenSync+readSync(fd, buf, 0, bytes, 0)which reads only the firstFILE_HEAD_BYTESbytes from disk.P2 — Truncate context command output at source
Original used
spawnSyncwithmaxBufferup to 64MB then sliced toCTX_HEAD_BYTESafterward. For high-volume commands (widegit diffranges) this blocks longer than the bash original's... | head -c 20000pipeline that short-circuits at the boundary. Reframed wrapped command as(cmd) 2>&1 | head -c <N>so the shell pipeline truncates at source.Test plan
bun --bun tsc --noEmitclean.eslintclean.🤖 Generated with Claude Code