Skip to content

Make the analysis web UI non-blocking - #1061

Closed
skerbis wants to merge 2 commits into
mainfrom
feature/non-blocking-analysis-page-v2
Closed

Make the analysis web UI non-blocking#1061
skerbis wants to merge 2 commits into
mainfrom
feature/non-blocking-analysis-page-v2

Conversation

@skerbis

@skerbis skerbis commented Sep 4, 2026

Copy link
Copy Markdown
Member

Summary

Stacked on #1060 (unrelated standalone bugfix, split out per review feedback) — please review/merge that one first.

pages/analysis.php previously ran RexStan::runFromWeb() synchronously, blocking the whole page request for however long the PHPStan run took (seconds to minutes on a larger codebase), with the attendant risk of hitting max_execution_time or a reverse-proxy timeout.

The page now loads instantly:

  • if a background run is already in progress, it shows a spinner and starts polling immediately;
  • otherwise it shows the last cached result (if any) plus a re-run button;
  • on the very first run on a system (no cache yet), it kicks off a background run automatically.

New pieces:

  • RexStanRunStore: file-based state (lock/result/error-log) shared between the detached background process and the polling requests. A lock older than 15 minutes is treated as an orphaned/crashed run rather than an active one, so a dead background process can never permanently block future runs.
  • RexStan::startBackgroundWebAnalysis(): spawns the same PHPStan invocation runFromWeb() already used, detached from the request (Unix: shell_exec('(...) &'), Windows: start /B). The result is written to a temp file first and renamed into place afterwards, so a poller can never observe a partially-written result.
  • Api\AnalysisApi: the ajax endpoint backing start/status polling, registered as rexstan_analysis. Mirrors the ob_start()+clean-JSON-response guard and the session_write_close()-before-long-running-work pattern used elsewhere in this ecosystem for the same reasons.
  • RexResultsRenderer::renderAnalysisBody(): the rendering logic that used to live directly in pages/analysis.php, extracted so both the page (cached result) and the status endpoint (fresh result) render identical markup.
  • assets/rexstan-analysis.js: vanilla JS driving the start/poll/swap flow.

Also extracted RexStan::interpretAnalysisOutput() (the JSON-vs-plain-text interpretation of PHPStan's raw output) out of runFromWeb() so both the synchronous and the new background path share it, and fixed a pre-existing PSR-3 log-interpolation finding on the line it moved.

Known limitations (addressed or tracked in follow-up PRs on top of this one)

  • No fallback to the old synchronous behavior on hosts where shell_exec()/proc_open() is unavailable.
  • No manual cancel of a running background analysis (only automatic stale-detection after 15 minutes).

Three follow-up PRs are stacked on top of this one, each fixing a specific issue found while building/testing it (per review feedback: one PR per bug rather than bundling), plus a final PR adding a priority-summary feature. All were originally reviewed together in #1059, now split up.

Test plan

  • Manual end-to-end test: start returns instantly, isRunning() correctly reflects state, double-start is rejected, background completion is detected, atomic rename verified.
  • Verified the background run produces byte-identical results to a direct, uncached phpstan analyse invocation (ruling out any shortcut/staleness).
  • php -l on all changed/added files; PHPStan on all new/changed files: 0 findings.

🤖 Generated with Claude Code

skerbis and others added 2 commits September 4, 2026 13:22
…output

RexStan::generateAnalysisBaseline() and analyzeSummaryBaseline() (used
by the "ignore all" button on the analysis page and by the summary
page respectively) invoke PHPStan without --no-progress, unlike every
other PHPStan invocation in this file.

Depending on environment, PHPStan's progress-bar control characters
end up interleaved into the captured stderr output. On failure that
raw output is placed directly into the thrown exception's message
("Unable to generate baseline: <progress bar escape codes>"), making
the actual underlying error unreadable - encountered this while a
Nette container-cache directory had gone stale, where the real cause
("Unable to create file ...") was buried under garbled control
characters.

Both call sites now pass --no-progress, matching every other PHPStan
invocation already in this file.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pages/analysis.php previously ran RexStan::runFromWeb() synchronously,
blocking the whole page request for however long the PHPStan run took
(seconds to minutes on larger codebases), with the attendant risk of
hitting max_execution_time or a reverse-proxy timeout.

The page now loads instantly:
- if a background run is already in progress, it shows a spinner and
  starts polling immediately;
- otherwise it shows the last cached result (if any) plus a "re-run"
  button;
- on the very first run on a system (no cache yet), it kicks off a
  background run automatically.

New pieces:
- RexStanRunStore: file-based state (lock/result/error-log) shared
  between the detached background process and the polling requests.
  A lock older than 15 minutes is treated as an orphaned/crashed run
  rather than an active one, so a dead background process can never
  permanently block future runs.
- RexStan::startBackgroundWebAnalysis(): spawns the same phpstan
  invocation runFromWeb() already used, detached from the request
  (Unix: `shell_exec('(...) &')`, Windows: `start /B`). The result is
  written to a temp file first and renamed into place afterwards, so a
  poller can never observe a partially-written result.
- Api\AnalysisApi: the ajax endpoint backing start/status polling,
  registered as "rexstan_analysis". Mirrors the
  ob_start()+sendJsonClean() stray-output guard and the
  session_write_close()-before-long-running-work pattern used
  elsewhere in this ecosystem for the same reasons.
- RexResultsRenderer::renderAnalysisBody(): the rendering logic that
  used to live directly in pages/analysis.php, extracted so both the
  page (cached result) and the status endpoint (fresh result) render
  identical markup.
- assets/rexstan-analysis.js: vanilla JS (no jQuery dependency)
  driving the start/poll/swap flow.

Also extracted RexStan::interpretAnalysisOutput() (the JSON-vs-plain-
text interpretation of PHPStan's raw output) out of runFromWeb() so
both the synchronous and the new background path share it, and fixed
a pre-existing PSR-3 log-interpolation finding on the line it moved.

Known limitations (see CHANGELOG): no manual cancel of a running
background analysis yet; no fallback to the old synchronous behavior
on hosts where shell_exec()/proc_open() is unavailable.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@skerbis
skerbis changed the base branch from fix/analyze-summary-baseline-no-progress to main September 4, 2026 11:31
@skerbis

skerbis commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Closing this and the 4 PRs stacked on top of it (#1062#1065) to flatten the 5-deep stack into a small set of independent PRs, as discussed with @staabm's feedback in mind.

This PR's content is unchanged and will reopen immediately as a fresh PR against main (same branch, so no rework), no longer tracked by GitHub's native stacked-PR feature that was blocking retargeting individual PRs. The two follow-ups will become siblings based on that new PR's branch instead of a serial chain.

@skerbis

skerbis commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

Reopened as #1067 (same content).

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.

1 participant