Skip to content

Process render overflow fix#1220

Merged
GT-610 merged 14 commits into
mainfrom
process-render-overflow-fix
Jun 30, 2026
Merged

Process render overflow fix#1220
GT-610 merged 14 commits into
mainfrom
process-render-overflow-fix

Conversation

@GT-610

@GT-610 GT-610 commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Resolve #1218

Also add issuers of #1218 to participants list

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Process details are now presented more clearly, separating command and arguments when available.
    • Process results can be sorted by different modes while keeping existing metadata consistent.
  • Bug Fixes

    • Improved parsing and formatting of ps output when the START column is absent.
    • More robust handling of unsupported/odd process headers, showing an error instead of failing.
    • Process auto-refresh is more reliable (prevents overlapping refreshes and better handles pause/resume and disconnected states).
    • Stop/kill actions now use the correct system-specific command behavior.
  • Tests

    • Added coverage for header variations, parsing, sorting, and error reporting.

GT-610 added 13 commits June 30, 2026 11:53
Distinguish a disconnected server from an empty result so the snackbar reflects the actual state, and log refresh failures with their stack trace for easier debugging.
Implement operator==/hashCode so ListView can skip rebuilding rows whose process data is unchanged between refreshes, reducing unnecessary widget rebuilds for idle processes.
Proc.toJson/toString and the Miscs import were never referenced by the GUI app, so drop them to keep the model lean.
Implement WidgetsBindingObserver so the periodic process refresh timer is cancelled when the app is paused and resumed (with an immediate refresh) when it returns to the foreground, avoiding wasted battery and traffic on background SSH commands. Also align InkWell borderRadius with CardX for consistent ripple clipping.
The previous index-based comparison treated loading (status parsing in progress) as eligible for running process commands, which could contend with the in-flight status request on the same SSH connection. Use explicit equality with connected/inished instead.
@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 92ba3867-f4a5-4e76-9122-db594fcafd13

📥 Commits

Reviewing files that changed from the base of the PR and between 36098c1 and b444d81.

📒 Files selected for processing (1)
  • lib/view/page/process.dart
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/view/page/process.dart

📝 Walkthrough

Walkthrough

The Unix ps script generation drops the START column and updates field parsing accordingly. Proc gains a derived args field, removes JSON/string serialization, and updates equality and hashing. PsResult.parse now handles unsupported headers without throwing and adds sortedBy(ProcSortMode). ProcessPage adds refresh guarding, lifecycle-based timer control, SSH gating, OS-specific kill commands, and a redesigned process row layout. Tests cover the new parsing, sorting, and error-handling behavior.

Possibly related PRs

Suggested reviewers

  • lollipopkit
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes unrelated changes, such as updating the GitHub participants list and removing Proc serialization helpers. Drop unrelated GitHub ID edits and keep only process-rendering fixes, or split nonessential Proc API cleanup into a separate PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: fixing the process list rendering bug.
Linked Issues check ✅ Passed The process page layout and ps parsing changes directly target the overlapping process-list text bug described in #1218.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch process-render-overflow-fix

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot requested a review from lollipopkit June 30, 2026 05:37

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/view/page/process.dart (1)

349-358: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Serialize stop commands with process refreshes.

A periodic _refresh() can already be awaiting client.run(ps) while Line 354 starts client.run(kill) on the same client; then Line 358 may immediately return because _isRefreshing is still true, leaving stale data. Reuse a shared process-command guard/mutex for kill and refresh, or block the stop action while refresh is active.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/view/page/process.dart` around lines 349 - 358, The stop action in the
onTap handler is racing with the periodic refresh because it calls
client.run(_killProcessCmd(...)) and then _refresh() without sharing the
existing refresh guard. Update the process control flow in page/process.dart so
the kill command and _refresh() use the same mutex/serialization mechanism as
_refresh and any client.run(ps) path, or make the stop action wait until refresh
completes before issuing the kill. Use the onTap callback, _refresh(),
_isRefreshing, and the client.run(...) call sites as the key places to wire in
the shared guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lib/data/model/server/proc.dart`:
- Line 56: The command parsing in Proc needs to handle quoted executable paths
before splitting binary and args. Update the binary/args derivation logic in
_parseArgs so it respects a quoted Windows path like "C:\Program
Files\App\app.exe" and does not split on the first literal space inside quotes.
Make sure the resulting binary is the full executable path, args contains only
the remaining switches, and any name/sort/display logic that depends on binary
is based on the parsed executable rather than the raw split tokens.

In `@lib/view/page/process.dart`:
- Around line 94-106: The _refresh flow is still showing the disconnected
snackbar every time the timer-driven refresh is skipped, which causes repeated
spam during filtered states. Update _refresh in Process page to silently return
when _canRunProcessCmd(serverState) is false, or gate the snackbar so it only
appears for explicit user-triggered refreshes, using the existing _refresh and
_canRunProcessCmd symbols to keep the periodic path quiet.
- Around line 415-418: The Windows branch in _killProcessCmd currently returns a
PowerShell-only command, which can fail when client.run() executes under a
cmd.exe SSH shell instead of PowerShell. Update the SystemType.windows case in
_killProcessCmd to use a cmd-compatible process termination command so it works
through the client.run() path. Keep the Linux/BSD branch unchanged and ensure
the new Windows command remains forceful and targets the given pid.

---

Outside diff comments:
In `@lib/view/page/process.dart`:
- Around line 349-358: The stop action in the onTap handler is racing with the
periodic refresh because it calls client.run(_killProcessCmd(...)) and then
_refresh() without sharing the existing refresh guard. Update the process
control flow in page/process.dart so the kill command and _refresh() use the
same mutex/serialization mechanism as _refresh and any client.run(ps) path, or
make the stop action wait until refresh completes before issuing the kill. Use
the onTap callback, _refresh(), _isRefreshing, and the client.run(...) call
sites as the key places to wire in the shared guard.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 4e7e3ede-0db8-437c-819c-53333f7f6b61

📥 Commits

Reviewing files that changed from the base of the PR and between 82bf08a and 36098c1.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • lib/data/model/app/scripts/script_builders.dart
  • lib/data/model/server/proc.dart
  • lib/data/res/github_id.dart
  • lib/view/page/process.dart
  • test/proc_test.dart

Comment thread lib/data/model/server/proc.dart
Comment thread lib/view/page/process.dart Outdated
Comment thread lib/view/page/process.dart
Gate the disconnected snackbar so only user-triggered refreshes show it, preventing spam from periodic timer ticks. Use 	askkill /F /PID for Windows kills since client.run() targets cmd.exe, not PowerShell. Serialize the stop action through the existing _isRefreshing guard via _killAndRefresh so the kill command never races with an in-flight ps fetch and the post-kill refresh is not skipped.
@GT-610 GT-610 merged commit 0e7b71b into main Jun 30, 2026
3 checks passed
@GT-610 GT-610 deleted the process-render-overflow-fix branch June 30, 2026 05:54
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.

进程bug

1 participant