Skip to content

fix(cli): show progress while patching findings - #931

Merged
mldangelo-oai merged 5 commits into
mainfrom
dev/ianw/patch-progress
Sep 17, 2026
Merged

mldangelo-oai merged 5 commits into
mainfrom
dev/ianw/patch-progress

Conversation

@ianw-oai

@ianw-oai ianw-oai commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

After findings are approved for patching, the CLI can be silent until each patch finishes. Show the current finding, elapsed time, and live Codex activity so users can follow the work.

Changes

  • Reuse the existing activity parser and timer for scan follow-up patches and saved findings.
  • Send progress to stderr and keep completed results in terminal history.
  • Preserve plain progress for headless scans, JSON scans, CI, and redirected output. Stop the timer on completion, failure, or interruption.
  • Merge current main and retain both the progress regressions and the newer validation-prompt tests.
  • Stop the timer before runner diagnostics and clip timer redraws to terminal columns using the existing Unicode display-width logic.
  • Emit completed reasoning summaries in terminal history and include baseline preparation and result collection in each finding’s timer.

Testing

On head 80ff324:

  • Focused CLI, patch, verification, and skill tests with seed 12345: 270 passed.
  • Regressions for runner diagnostics, long ASCII/Unicode titles, streamed reasoning summaries, and baseline preparation fail before the corrections and pass afterward.
  • pnpm run types: passed.
  • pnpm run build:ci: passed.
  • pnpm run format: passed.
  • git diff --check origin/main...HEAD: passed.

The checks above use the SDK dependencies pinned by this branch. See GitHub checks for full-suite and platform results. No live model scan was run.

Risk and rollout

Low risk. This changes terminal progress using existing Codex events and flags. Progress display failures do not stop patching. No new public CLI options are introduced.

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.

@github-actions github-actions Bot added the bug Something isn't working label Sep 15, 2026
@ianw-oai
ianw-oai marked this pull request as ready for review September 15, 2026 21:02
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 15, 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-09-17T21:43:55.926155Z 80ff324 Manual request
🔒 Security Review ✅ Completed 2026-09-17T21:43:48.166221Z 80ff324 New commits
ℹ️ 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.

Copy link
Copy Markdown

I rebased and locally verified this change against current main at 02c5137fbe807a83c370bed05dc2a8d06f113d79.

The only merge conflict was in sdk/typescript/tests-ts/cli-patch.test.ts; I retained both the current-main test additions and this PR's progress tests. The resulting candidate changes the same three files and preserves the intended behavior.

Verification on the rebased candidate:

  • the three new progress regressions fail on the baseline and pass on the candidate;
  • bun test --timeout 30000 --seed 12345 tests-ts/cli-patch.test.ts tests-ts/cli-verify-fix.test.ts tests-ts/cli-skills.test.ts: 111 passed, 0 failed;
  • pnpm run types, pnpm run build:ci, and changed-file Prettier checks passed;
  • Git whitespace and post-test tracked-file checks passed.

The focused suite was run; the full upstream suite and live cloud-agent execution were not run. No upstream branch, push, PR update, or merge was performed by this verification.

Copy link
Copy Markdown

I opened #950 with this PR rebased onto current main and the cli-patch.test.ts conflict resolved. It preserves the original implementation and tests; the new PR includes the focused local validation results.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 61221098a6

ℹ️ 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".

Comment thread sdk/typescript/src/cli.ts Outdated
Comment thread sdk/typescript/src/cli.ts

@mldangelo-oai mldangelo-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 refreshed head. The merge retains both the patch-progress regressions and the newer validation-prompt tests, and the implementation continues to reuse the existing presenter and timer. All 111 focused tests, types, build, and formatting pass. No blocking findings.

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e91c2e70f

ℹ️ 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".

Comment thread sdk/typescript/src/cli.ts Outdated
Comment thread sdk/typescript/src/cli.ts
@mldangelo-oai

Copy link
Copy Markdown
Collaborator

@codex review

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 80ff3246de

ℹ️ 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".

@mldangelo-oai mldangelo-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 final head 80ff324. The four previous findings are fixed: runner diagnostics start after the timer, long titles fit terminal columns, reasoning summaries print once, and progress covers preparation through final file comparison. All 270 focused tests, types, build, formatting, and whitespace checks pass locally; Codex review reports no major issues on this head. No blocking findings. Full platform CI remains a separate merge gate.

@mldangelo-oai
mldangelo-oai merged commit ef298ca into main Sep 17, 2026
55 checks passed
@mldangelo-oai
mldangelo-oai deleted the dev/ianw/patch-progress branch September 17, 2026 21:56
@github-actions github-actions Bot mentioned this pull request Sep 18, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants