Skip to content

🐛 Keep foreground command output inside its REPL entry - #1093

Merged
taras merged 3 commits into
mainfrom
codex/repl-exec-output-1026
Nov 3, 2026
Merged

taras merged 3 commits into
mainfrom
codex/repl-exec-output-1026

Conversation

@taras

@taras taras commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

Why

Ordinary unbound sh exec output bypasses the interactive REPL and can overwrite its panes. Fixes #1026.

What changes

Stdout and stderr stream into labelled command sections within the producing entry, including output received during cleanup. Output arrival preserves selection, focus and drafts.

Entry-scoped downstream stdio handlers preserve core capture, hidden-output, retention and exit-status behavior. Command presentation stays separate from document Output. Independent streaming decoders preserve split Unicode. Control characters become readable inert text before fitting and drawing, including captured output shown in both the center reading and its associated Sidekick display.

Review guide

Start with packages/cli/src/repl/session.ts, then the display path through program.ts, application.ts and source-reading.ts. Check terminal ownership, per-entry attribution through teardown, visibility and UI cost as content grows. The REPL specification records the boundary.

How to verify it

Final signed feedback head: 0297664daee4d91b0ecb3f264f5f704a194f6348. It integrates current main's #1064 merge without changing any of the 8 exec-fix blobs or 51 incoming blobs.

  • On the final combined tree, deno task test packages/cli/tests/repl-exec-output.test.ts packages/cli/tests/repl-sidekick-presentation.test.ts packages/cli/tests/adversarial-implementation.test.ts passes 26 suites and 110 steps.
  • The acceptance file covers 8 steps: mounted live streams, capture/hidden visibility, Unicode and controls, resize, held failed/cancelled cleanup, history/Sidekick attribution, cold inspection, and identical readable controls in both associated displays.
  • The original focused reading/lifecycle/history selection passes 72 steps; core foreground and ordinary CLI retention pass 42 steps. Their original receipts remain attributed to the earlier candidate, with applicability through unchanged affected inputs.

Each command carries the existing prepared DENO_DIR so isolated-HOME subprocesses use the prepared cache. The final mounted E1 test fails unchanged with routing disabled and passes with it enabled. The associated-display regression also fails before its correction and passes with identical test bytes after it. First failures and candidate receipts are retained locally. CI owns exhaustive runtime checks.

Reviews and delivery

Original Planner acceptance and the independent Fable exact-head review both pass 0297664daee4d91b0ecb3f264f5f704a194f6348. Fable retains four nonblocking cost, labeling and comment observations. Required CI and current exact-main health remain merge gates.

The bounded XMD REPL experiment completed 7 coordinated role requests with one source-correction round and no automatic provider or response retry. Current-main integration and delivery-status reads use no model. Existing automatic CI review workflow runs are tracked separately.

Risks and limitations

Visible forwarded presentation lasts for this REPL process. Cold viewing uses the existing saved per-command stdout/stderr/status and starts no command. The existing upstream process-pump tail limitation remains.

Scope confirmation

  • Every changed file supports foreground output containment or its evidence/specification.
  • No unrelated cleanup, dependencies or generated artifacts.
  • Description reflects the current committed behavior and focused evidence.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

PR #1093: 🐛 Keep foreground command output inside its REPL entry

8 files, +658 / -13

Scope

🟡 671 lines changed. PRs under 400 receive more thorough review.

Structural

Oxlint structural signals:

  • no-unused-vars ×15: packages/cli/src/repl/session.ts, packages/cli/src/repl/program.ts, packages/cli/src/repl/application.ts
  • no-unnecessary-type-arguments ×2: packages/cli/src/repl/session.ts
  • no-unnecessary-boolean-literal-compare ×1: packages/cli/src/repl/application.ts

Slop

✅ Slop indicators look low.

Static Analysis

Oxlint: 155 diagnostics across 5 files (12 rules)
Density: 0.236 violations/added-line

no-shadow (91): packages/cli/src/repl/source-reading.ts, packages/cli/src/repl/session.ts, packages/cli/src/repl/program.ts (+1)
no-floating-promises (19): packages/cli/src/repl/session.ts, packages/cli/src/repl/program.ts
no-unused-vars (15): packages/cli/src/repl/session.ts, packages/cli/src/repl/program.ts, packages/cli/src/repl/application.ts
consistent-function-scoping (7): packages/cli/src/repl/application.ts
unbound-method (6): packages/cli/src/repl/program.ts, packages/cli/src/repl/session.ts
consistent-return (6): packages/cli/src/repl/source-reading.ts, packages/cli/src/repl/application.ts, packages/cli/src/repl/program.ts
no-useless-default-assignment (3): packages/cli/src/repl/application.ts, packages/cli/src/repl/program.ts
no-duplicate-type-constituents (3): packages/cli/src/repl/application.ts
no-unnecessary-type-arguments (2): packages/cli/src/repl/session.ts
no-control-regex (1): packages/cli/src/repl/command-output.ts
no-array-sort (1): packages/cli/src/repl/application.ts
no-unnecessary-boolean-literal-compare (1): packages/cli/src/repl/application.ts

Correctness

No extraneous code patterns detected.

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Independent Fable review: PASS

Exact commit: 26ee453ea6a23a7100876433a53d5303bf7289e1. Provider claude, requested model alias fable, effort high. One original independent call completed and joined; no retry or length exception. This is the saved verdict, published by the coordinator.

Reviewed 26ee453 against 9b98cbe. Routing is correct: runEntry installs per-entry Stdio handlers at min before agent/installation/execution resources, and context-api orders scope min handlers downstream of core's per-exec max boundary, so handlers receive only bytes core forwarded after retention/bind/capture/hidden routing and never call next, so no physical write occurs; an outer test-scope min observer sits further downstream, which makes E1's leak check valid and the routing-disabled fail-before witness meaningful. Retained exec records, bindings and root output are untouched (core unchanged). Attribution uses the captured entry key; the frozen-prefix ReplLive in viewFor omits commandOutput, so frozen positions show none while a live older selection keeps its own; arrival wakes via outputs.send without touching state. Decoders are per-entry and flushed in an ensure registered before the handlers. Control escaping is a display-only copy at plain(). Cold reopen path unaffected (fresh process, saved records only). Spec and xmd run contracts hold. Only non-blocking cost and tidiness observations remain.

All six original acceptance clauses were examined: live-renderer, capture-retention, terminal-safety, lifecycle-attribution, cold-history, run-compatibility.

Nonblocking observations:

  1. packages/cli/src/repl/session.ts:1021 session.commandOutput rebuilds frozen arrays for every in-process entry on each read, and program.ts calls liveOf twice per cycle (reduceRepl at 1662 and viewFor at 3021) before application.ts:6152 selects one entry. Because append merges consecutive same-channel text, cost is O(entries x channel alternations), not O(bytes), so it is small; a ReadonlyMap or a snapshot cached until the next append would remove the per-frame copy. Unmeasured; no evidence of a user-visible regression.

  2. packages/cli/src/repl/source-reading.ts:279 plain() reruns inertCommandText and split over the selected entry's whole command text and reallocates every ReplReadingLine per frame while a command streams; the fit store caches cuts by text so measurement is not repeated, and the existing live Output path has the same shape. Process-local retention of every entry's command text for the session lifetime is per the accepted plan. Unmeasured growth concern only.

  3. packages/cli/tests/repl-source-reading.test.ts:707 the comment 'two captioned sections' is stale now that the reading has output, commands and source keys. The expectation change itself is disclosed with its retained first failure and is required by the shape change.

  4. Any @effectionx/process child pumped inside the entry scope (document daemons, installation children) is presented under the 'Command stdout/stderr' labels of that entry. This is consistent with containment and attribution to the producing round; noted so the labels are not read as exec-only.

This verdict covers the exact commit above. Required CI and current exact-main health remain delivery gates.

@taras
taras marked this pull request as ready for review November 3, 2026 19:34

@taras taras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Independent Fable exact-head follow-up: PASS

Commit 0297664daee4d91b0ecb3f264f5f704a194f6348. Original independent conversation retained; provider claude, requested alias fable, effort high. One follow-up after the finite correction and current-main integration; no automatic retry or length exception. The coordinator is publishing the saved verdict.

Inspected merge 0297664 (tree 380fa8d5) with parents ba71128 and main 98cc6f1; merge-base is 9b98cbe. The merge is disjoint: the eight fix files are byte-identical to ba71128, all 51 incoming files are byte-identical to main, and main touched no repl/, core foreground/execute, test-support, spec or config path. The tree equals the tested candidate tree in the focused receipt, and the committed acceptance test hash equals the identical before/after hash. The correction is one line: workComposed in application.ts applies the existing inertCommandText to the display copy of entry.terminal.output before splitting, marker prefixing and fitting, matching the center reading's plain() conversion; retained output, bindings and exec records are untouched, and the new mounted regression distinguishes the base. Remaining consumers of entry output are the chat preparation facts list (cache identity only) and program.ts live.output, which is the Sidekick turn's own reply text, so no unconverted command display remains. My earlier routing, ordering, attribution, frozen-prefix and cold-history traces are unchanged at this head. The four prior non-blocking findings still apply; nothing blocks.

All six original acceptance clauses examined: live-renderer, capture-retention, terminal-safety, lifecycle-attribution, cold-history, run-compatibility.

Nonblocking observations:

  1. packages/cli/src/repl/session.ts session.commandOutput rebuilds frozen arrays for every in-process entry per read, and program.ts reads it twice per cycle (reduceRepl and viewFor) before application.ts readingOf selects one entry. Cost is O(entries x channel alternations) because same-channel text merges, so it is small; a ReadonlyMap or a snapshot cached until the next append would remove the copy. Unmeasured, unchanged at this head.

  2. packages/cli/src/repl/source-reading.ts plain() reruns inertCommandText and split over the selected entry's whole command text per frame while streaming, allocating every ReplReadingLine; the fit store caches cuts by text so measurement is not repeated, and the existing live Output path has the same shape. The new workComposed conversion runs only on chat re-preparation since the preparation facts key on the raw output. Unmeasured growth concern only.

  3. packages/cli/tests/repl-source-reading.test.ts:707 the comment 'two captioned sections' remains stale after the key expectation became output, commands, source.

  4. Any @effectionx/process child pumped inside the entry scope (document daemons, installation children) is presented under the entry's 'Command stdout/stderr' labels. Consistent with containment and producer attribution; noted so the labels are not read as exec-only.

Original Planner acceptance also passed this exact commit. Required CI and current exact-main health remain merge gates.

@taras
taras enabled auto-merge (squash) November 3, 2026 21:40
@taras
taras merged commit 3d7349a into main Nov 3, 2026
56 of 57 checks passed
@taras
taras deleted the codex/repl-exec-output-1026 branch November 3, 2026 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep command output inside its round in xmd repl

1 participant