Skip to content

refactor(sdk): simplify CLI completion and display state - #1384

Open
mldangelo-oai wants to merge 1 commit into
mainfrom
mdangelo/codex/simplify-cli-terminal-state
Open

mldangelo-oai wants to merge 1 commit into
mainfrom
mdangelo/codex/simplify-cli-terminal-state

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The CLI tracks child-process completion and terminal updates with state that duplicates existing mechanisms. Remove those extra flags and bookkeeping while preserving completion, cancellation, terminal cleanup, and displayed output.

Changes

Rely on a Promise settling only once for child completion and errors. Use the existing stream-generation counter to identify the active progress session during listener cleanup. Remove redundant dashboard state updates, write frames directly, and use the remaining first-line prefix to track history indentation.

Testing

In the recorded focused run, a real child-process fixture ignores SIGTERM and verifies that CLI cancellation still settles with exit status 143. Writable-stream fixtures inject asynchronous output failures and check that error listeners remain until output settles, then are removed. Dashboard fixtures check terminal restoration after rendering/setup failures, and history fixtures check visible content at narrow and wide widths.

After building the bundled plugin, repeat from sdk/typescript with bun test tests-ts/cli.test.ts tests-ts/cli-skills.test.ts tests-ts/scan-dashboard.test.ts tests-ts/scan-history-renderer.test.ts. The SIGTERM fixture is POSIX-only.

Risk and rollout

No CLI syntax or output-format changes are intended. Completion and stream cleanup must remain correct when events arrive after cancellation or after progress restarts.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-07T03:24:51.643293Z d9eace5 PR opened
🔒 Security Review ✅ Completed 2026-10-07T03:25:53.536867Z d9eace5 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kmbroai kmbroai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the complete lifecycle and listener changes, including completion, errors, cancellation, forced termination, dashboard cleanup, and history wrapping. All 259 focused tests passed locally; current CI is passing. No actionable findings at this commit.

@zcrab-oai zcrab-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the diff and relevant surrounding code at d9eace5. No new correctness or regression issues found. Focused helper checks passed; the full suite was not rerun locally.

This branch has not been deployed

No deployments
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.

4 participants