You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(cli): refresh scan progress from saved state - #1757
Local standard and diff scans can remain on “preflight” when model output contains no new progress markers. Poll saved scan state so the CLI updates its phase and review counts while the scan runs.
Changes
Reuse the Deep Scan progress tracker for saved phase and coverage updates, with cancellation and nonfatal read failures.
Show review counts separately from file counts in terminal and headless output. Keep worker-local phases from advancing the whole scan.
Preserve hosted progress and follow-up worker notifications, update the hosted event schema, and include the renamed module in the npm package.
Isolate the hosted-progress test from machine-specific Python discovery. Cover polling cleanup before follow-up turns after successful and failed primary scans, and before repository reconciliation even without a follow-up prompt.
Testing
Full SDK API suite: 256 tests passed; four Windows-only cases skipped on macOS. The six reconciliation scenarios reproduced active polling before the fix and pass with it.
Saved scan and Deep Scan progress suites: 16 tests and 24 assertions passed.
SDK and MCP type checks, plugin bundle build, changed TypeScript formatting, and diff checks passed.
Fresh hosted CI and Codex/Copilot reviews will validate the updated head before merge.
Risk and rollout
Adds one saved-state read every five seconds for local standard and diff scans with a progress observer. Polling stops at completion or cancellation and before separate validation, follow-up turns, or repository reconciliation. Existing file-count fields remain available; review counts are optional. No command or flag changes.
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.
The reason will be displayed to describe this comment to others. Learn more.
Owner-level merge-safety review of b254f1e6a1839b514c685dd4742dbf1527e79be7 completed across all 17 changed files and the workbench/CLI/hosted callers. Reusing the poller and keeping review counts separate from file coverage addresses the missing-marker behavior. I found no actionable issue.
Builds, types and formatting passed, as did 156 tracker/parser/dashboard tests and the installed archive probe for module loading, CLI output and review-only updates. Five focused API/validation cases and protected hosted/package fixtures hit the existing local filesystem restrictions. The hosted merge has an identical tree and passes the full Node24 installed-package smoke, including hosted and real Deep Scan lifecycle checks; two Ubuntu shards pass 2,976 tests. Other CI shards are still running, so this is not a claim that the complete suite has passed.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. I completed an owner-level merge-safety and simplification review of 1590c428d956aa3e420f453f51516d4e5f28e106 and found no actionable issues. The test isolation and schema-test extraction preserve the reviewed progress behavior.
Fresh builds, types, formatting, source checks, 156 progress/dashboard tests, 179 Python tests (plus 148 subtests), and the rebuilt installed-package progress probe passed. Six focused API/CLI cases passed; four hit the local filesystem ownership restriction. Those four pass in the successful CI merge run, which also validates the installed-package lifecycle. That run includes newer main changes, so it is integration evidence rather than an identical-head suite run.
Resolved the main conflict by retaining the schema and writeup-recovery tests already extracted on main, removing the duplicate extraction. The progress implementation merged cleanly. Included the reviewed shared fixture repairs from #1763.
Passed locally: 182 Python tests and 151 subtests, formatting of merged SDK files, direct source compatibility checker, and diff checks. SDK tests could not start because locked dependencies are unavailable; the five portable commands were attempted in order but pinned Ruff/dependency setup remains blocked. Full hosted CI is required before merge.
The reason will be displayed to describe this comment to others. Learn more.
I completed an owner-level merge-safety review of b6b5891c4643af41854904d85a2cba9af47bffd0. I would fix the existing failure-path follow-up issue before merging. When the primary scan fails, a five-second poll or an already pending read can still emit scan progress during postScanPrompt. Stop the tracker before that follow-up and add a failed-primary-scan lifecycle test.
A targeted probe using the exact failure-follow-up block and real tracker reproduced both cases; stopping first suppressed both. This was not a full API end-to-end run. I found no additional actionable issues.
Types, formatting, builds, portable source checks, 156 progress/dashboard tests, 99 publication tests, and the rebuilt installed-package progress probe passed. Full local SDK runs did not pass: protected fixtures hit the environment ownership restriction, and broader runs stopped at a cancellation assertion and temporary Git fixture failure. Current CI is still running.
Please review 11c6316317c68fd9a47921c34e2a862dfa66875f, which addresses the failure-path progress-polling lifecycle issue raised in review. The follow-up now stops the tracker before its first turn, with regression coverage for successful and failed primary scans. Hosted CI remains required before merge.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Saved-state polling remains active during post-completion matching when no earlier stop occurs.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Stop scan polling before post-scan finding reconciliation
sdk/typescript/src/api.ts:2765
Without a post-scan prompt (or an earlier custom-validation stop), polling continues into repository finding reconciliation at lines 2858–2907. Matching can run additional model turns, so the completed scan keeps launching get-scan reads every five seconds until matching finishes. Move this stop before the conditional so polling ends before post-scan work even when no follow-up prompt is configured.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the failed-scan follow-up. I completed an owner-level merge-safety and simplification review of 11c6316317c68fd9a47921c34e2a862dfa66875f and found no material unresolved issue. The failure path now stops polling before post-scan instructions and preserves the primary failure.
The failed-primary API regression passed. A separate probe of the exact failure-follow-up block with the real tracker also suppressed timer and in-flight updates. Fresh builds, types, formatting, portable checks, 156 progress/dashboard tests, 190 Python tests (plus 183 subtests), and rebuilt installed-package progress/CLI checks passed.
Four focused completion/hosted cases and the custom-validation progress case hit the local filesystem ownership guard. Broad local suites did not pass: they stopped at that guard and a Cloud-publication cancellation assertion. Current CI remains pending, so this approval does not claim a complete suite pass.
Integrated main in a51aab6 after Windows shard 1 reached the old 25-minute job timeout. This brings in the merged 40-minute CI budget and Windows credential handling without changing the progress implementation. The earlier polling-cleanup review thread is resolved. Updated-file formatting and diff checks pass; fresh hosted CI is pending.
Addressed the earlier Copilot overview finding: saved-state polling now stops before repository reconciliation even when no post-scan prompt is configured. All six existing reconciliation scenarios reproduced the bug before the one-line lifecycle change.
The full API suite passes with 256 tests and four Windows-only skips; 16 progress tests also pass. SDK/MCP type checks, plugin build, formatting, and diff checks pass. Please review the updated lifecycle and regression coverage.
The reason will be displayed to describe this comment to others. Learn more.
No blocking findings in this owner-level merge-safety and simplification review of all 17 changed files. Thanks for stopping saved-state polling before repository reconciliation even when no follow-up prompt is configured. Successful and failed follow-ups, custom validation, and hosted progress remain consistent.
Fresh validation passed 438 affected tests in fixed-seed and randomized orders (four platform skips), including the full 256-test API suite, plus 13 independent lifecycle/concurrency probes. Builds, types, formatting and portable source checks passed.
The full SDK suite was not rerun locally. Current merge CI passes Linux/macOS test shards and installed-package checks; four Windows shards remain pending. That merge includes newer main changes.
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
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.
alandelong-oai
left a comment
Summary
Local standard and diff scans can remain on “preflight” when model output contains no new progress markers. Poll saved scan state so the CLI updates its phase and review counts while the scan runs.
Changes
Testing
Risk and rollout
Adds one saved-state read every five seconds for local standard and diff scans with a progress observer. Polling stops at completion or cancellation and before separate validation, follow-up turns, or repository reconciliation. Existing file-count fields remain available; review counts are optional. No command or flag changes.
Public disclosure review