Support terminal UI profiling - #6
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughIntroduces terminal UI profiling support via a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 55 minutes and 1 second.Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dart (1)
121-130:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't treat timed-out
getIsolatelookups as vanished isolates.With the new timeout on Lines 122-124, this catch now also swallows
TimeoutException. Removing that isolate fromliveAppIsolateIdslets the later length check reportallPaused: trueeven though the isolate may still be alive and merely slow to answer.Suggested fix
try { final isolate = await vmService .getIsolate(isolateRef.id!) .timeout(_vmServiceExitPauseRequestTimeout); if (isolate.pauseEvent?.kind == EventKind.kPauseExit) { pausedExitIsolateIds.add(isolateRef.id!); } + } on TimeoutException { + completeExitPauseSignal(allPaused: false); + return; } catch (_) { // The isolate can disappear while the VM is shutting down. liveAppIsolateIds.remove(isolateRef.id!); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dart` around lines 121 - 130, The catch currently treats any error (including TimeoutException from vmService.getIsolate().timeout) as the isolate having vanished and removes isolateRef.id from liveAppIsolateIds; change the error handling in the block around vmService.getIsolate/isolateRef/id/_vmServiceExitPauseRequestTimeout so that TimeoutException is handled separately (do not remove the id and simply skip/continue or log a timeout) while only non-timeout failures (e.g., RPC errors or not-found) remove the id; update the catch to distinguish TimeoutException (or TimeoutException type from dart:async) from other exceptions and preserve the existing removal behavior only for the latter, leaving pausedExitIsolateIds logic unchanged.packages/devtools_profiler_core/lib/src/capture/profile_runner.dart (1)
74-85:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStart the interrupt watcher before any awaited startup work.
handleInterruptSignalsis only enabled afterlaunchProfiledProcess()returns. If the user hits Ctrl+C/SIGTERM during process launch, port reservation, or any other awaited startup step before Line 83, the profiler misses the signal and falls back to abrupt termination instead of returning partial diagnostics.🔧 Suggested fix
- launchedProcess = await launchProfiledProcess( + interruptWatcher = request.handleInterruptSignals + ? _ProfileRunSignalWatcher.start() + : null; + + launchedProcess = await launchProfiledProcess( request: request, command: command, sessionId: sessionId, dtdUri: dtdSession.info.localUri.toString(), workingDirectory: workingDirectory, ); process = launchedProcess.process; sessionController.childProcessId = process.pid; - interruptWatcher = request.handleInterruptSignals - ? _ProfileRunSignalWatcher.start() - : null;If you want full coverage, move it even earlier so Ctrl+C also works during service registration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/devtools_profiler_core/lib/src/capture/profile_runner.dart` around lines 74 - 85, Move starting of the interrupt watcher so it runs before any awaited startup work: check request.handleInterruptSignals and call _ProfileRunSignalWatcher.start() and assign to interruptWatcher before calling await launchProfiledProcess(...); then call launchedProcess = await launchProfiledProcess(...) and only after it returns set process = launchedProcess.process and sessionController.childProcessId = process.pid as before. Ensure the watcher variable is nullable/cleared as currently implemented and that starting it early does not depend on the launched process being available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/devtools_profiler_cli/README.md`:
- Around line 69-82: Replace the placeholder example command in the README's
"Profile a terminal UI app" section with an executable, copy-pasteable fixture
command: update the --cwd value and the dart entrypoint so the sample uses the
repo's runnable TUI fixture (use
packages/devtools_profiler_core/test/fixtures/profiled_app as the --cwd and dart
run bin/artisanal_widget_app.dart as the command to run) while keeping the
--terminal flag and surrounding explanation intact.
In `@packages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart`:
- Around line 517-548: The probing loop in _completeKnownVmServiceUri can hang
indefinitely if the target neither exposes the VM service nor exits; add an
overall timeout parameter (e.g., Duration vmServiceTimeout, defaulting to a
sensible value or passed from the request) and enforce it by computing a
deadline before the loop and checking elapsed time each iteration (alongside the
existing processExited check and _canConnectToVmService call); if the deadline
is exceeded and serviceUri is not completed, complete it with a
TimeoutException/StateError describing the timeout so callers can handle the
failure. Ensure the new timeout is threaded from the caller into
_completeKnownVmServiceUri and update any callers to pass vmServiceTimeout, and
keep existing behavior of completing with expectedVmServiceUri when
_canConnectToVmService succeeds and still honoring exitCodeFuture.
- Around line 499-505: Add a comment to _reserveLoopbackPort explaining the
TOCTOU race: state that binding-then-closing a ServerSocket to get a free
loopback port introduces a small window between server.close() and the target
process binding where another process could take the port, that this is an
accepted tradeoff for this profiling tooling, and suggest alternatives (e.g.,
letting the target pick an ephemeral port or using OS-specific reservation APIs)
for callers that need stronger guarantees; place the note directly above the
Future<int> _reserveLoopbackPort() declaration so maintainers see the limitation
when inspecting that function.
- Around line 266-294: The private helper _dartLaunchPlan lacks documentation
explaining key profiler/VM-service assumptions; add a dartdoc comment above the
_dartLaunchPlan function that (1) describes why '--pause-isolates-on-exit=false'
is used when processIoMode == ProfileProcessIoMode.inheritStdio and why the flag
is set (implicit true) for pipe mode (explain impact on isolate lifecycle and VM
service connection), and (2) documents the conditional use of
Platform.resolvedExecutable vs command.first (i.e., only substitute when
normalizedExecutableName(command.first) == 'dart' and command.first == 'dart' to
ensure using the running SDK's dart binary). Also mention expectedVmServiceUri
behavior and the rationale for '--disable-service-auth-codes' when inheriting
stdio; reference the symbols _dartLaunchPlan, ProfileProcessIoMode,
normalizedExecutableName, and CommandLaunchPlan in the comment.
In
`@packages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dart`:
- Around line 8-10: Replace the fixed Timer in interrupt_parent.dart with a
readiness handshake: remove the Timer(const Duration(milliseconds: 1200), ...)
usage and instead listen on stdin (or read a line) for a specific "READY" token,
then call Process.killPid(parentPid, ProcessSignal.sigint) when that token is
received; keep the parentPid symbol as the target PID and update the test
harness to emit the "READY" line from the profiler under test when it has
finished startup so the fixture reliably triggers the interrupt only after
readiness.
---
Outside diff comments:
In `@packages/devtools_profiler_core/lib/src/capture/profile_runner.dart`:
- Around line 74-85: Move starting of the interrupt watcher so it runs before
any awaited startup work: check request.handleInterruptSignals and call
_ProfileRunSignalWatcher.start() and assign to interruptWatcher before calling
await launchProfiledProcess(...); then call launchedProcess = await
launchProfiledProcess(...) and only after it returns set process =
launchedProcess.process and sessionController.childProcessId = process.pid as
before. Ensure the watcher variable is nullable/cleared as currently implemented
and that starting it early does not depend on the launched process being
available.
In
`@packages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dart`:
- Around line 121-130: The catch currently treats any error (including
TimeoutException from vmService.getIsolate().timeout) as the isolate having
vanished and removes isolateRef.id from liveAppIsolateIds; change the error
handling in the block around
vmService.getIsolate/isolateRef/id/_vmServiceExitPauseRequestTimeout so that
TimeoutException is handled separately (do not remove the id and simply
skip/continue or log a timeout) while only non-timeout failures (e.g., RPC
errors or not-found) remove the id; update the catch to distinguish
TimeoutException (or TimeoutException type from dart:async) from other
exceptions and preserve the existing removal behavior only for the latter,
leaving pausedExitIsolateIds logic unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb8ebe5a-a108-4d68-82ff-2db13dc15f54
⛔ Files ignored due to path filters (1)
packages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
README.mdpackages/devtools_profiler_cli/CHANGELOG.mdpackages/devtools_profiler_cli/README.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/lib/src/presentation/cli_command.dartpackages/devtools_profiler_cli/lib/src/presentation/preparation.dartpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/README.mdpackages/devtools_profiler_core/lib/src/capture/profile_run_request.dartpackages/devtools_profiler_core/lib/src/capture/profile_run_result.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_controller.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dartpackages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_core/test/profile_runner_test.dartskills/devtools-profiler-local/SKILL.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Follow idiomatic Dart and keep code easy to scan in split-screen views
Prefer multi-line strings over string concatenation for large text blocks, command output fixtures, JSON examples, and terminal snapshots
Keep lines near 80 characters where practical. Long identifiers and URLs may exceed that when wrapping would make the code harder to read
Use records for short-lived grouped return values instead of introducing one-off classes
Use patterns,if-case, and switch expressions when they make parsing or dispatch logic clearer
Use class modifiers such assealed,final,base, andinterfacewhen they describe the intended inheritance boundary
Use digit separators for large numeric literals, for example timeouts, sample counts, and byte sizes
Use wildcard variables for intentionally unused callback parameters
Use null-aware collection elements when conditionally including nullable values in list or map literals
Use dot shorthands only when the inferred type is obvious from context
Use///documentation comments for public APIs
Start doc comments with a short, single-sentence summary
Put a blank line after the first sentence when adding more detail in doc comments
Avoid repeating information that is already obvious from the declaration in documentation comments
Start method comments with third-person verbs, such as 'Returns', 'Starts', or 'Captures'
Start non-boolean property comments with a noun phrase
Start boolean property comments with 'Whether'
Use square brackets for in-scope identifiers, such as [ProfileRunRequest], [Duration], and [StateError]
Explain parameters, return values, and exceptions in prose rather than using tag-style documentation
Prefer fenced Markdown code blocks for examples in documentation comments
Keep Markdown simple; avoid HTML in documentation comments
Files:
packages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_cli/lib/src/presentation/cli_command.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_controller.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/lib/src/capture/profile_run_result.dartpackages/devtools_profiler_core/lib/src/capture/profile_run_request.dartpackages/devtools_profiler_cli/lib/src/presentation/preparation.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
packages/devtools_profiler_*/**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
Consider documenting private helpers when they encode profiler behavior, artifact contracts, protocol semantics, or VM-service assumptions
Files:
packages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_cli/lib/src/presentation/cli_command.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_controller.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/lib/src/capture/profile_run_result.dartpackages/devtools_profiler_core/lib/src/capture/profile_run_request.dartpackages/devtools_profiler_cli/lib/src/presentation/preparation.dartpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
packages/devtools_profiler_**/pubspec.yaml
📄 CodeRabbit inference engine (AGENTS.md)
packages/devtools_profiler_**/pubspec.yaml: Do not add Flutter UI, web UI, or browser-only runtime dependencies to the profiler packages
Use hosteddevtools_sharedfor shared VM and memory models; do not vendor package trees into this workspace
Files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yaml
packages/devtools_profiler_core/pubspec.yaml
📄 CodeRabbit inference engine (AGENTS.md)
devtools_profiler_coremay depend onvm_service,dtd, anddevtools_shared
Files:
packages/devtools_profiler_core/pubspec.yaml
packages/*/README.md
📄 CodeRabbit inference engine (AGENTS.md)
Package READMEs should explain how that package is used and how it fits into the profiler system. Prefer examples that agents can execute directly from the CLI
Files:
packages/devtools_profiler_core/README.mdpackages/devtools_profiler_cli/README.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep the root
README.mdend-user focused
Files:
README.md
packages/devtools_profiler_cli/pubspec.yaml
📄 CodeRabbit inference engine (AGENTS.md)
packages/devtools_profiler_cli/pubspec.yaml: Keepdart_mcpon the supported^0.5.0line unless the user asks for an upgrade
devtools_profiler_climay depend on terminal/MCP/presentation packages such asartisanalanddart_mcp
Files:
packages/devtools_profiler_cli/pubspec.yaml
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_cli/pubspec.yaml : `devtools_profiler_cli` may depend on terminal/MCP/presentation packages such as `artisanal` and `dart_mcp`
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: For Flutter targets, remember that release mode, AOT builds, and browser/web targets do not expose the Dart VM service needed by this profiler
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_cli/lib/**/command*.dart : Split large CLI commands into focused files instead of growing a monolithic command file
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_*/**/*.dart : Consider documenting private helpers when they encode profiler behavior, artifact contracts, protocol semantics, or VM-service assumptions
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_cli/pubspec.yaml : Keep `dart_mcp` on the supported `^0.5.0` line unless the user asks for an upgrade
Applied to files:
packages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_cli/CHANGELOG.mdpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_core/CHANGELOG.md
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_cli/pubspec.yaml : `devtools_profiler_cli` may depend on terminal/MCP/presentation packages such as `artisanal` and `dart_mcp`
Applied to files:
packages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_cli/lib/src/presentation/cli_command.dartpackages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_cli/CHANGELOG.mdpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartREADME.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_cli/README.mdskills/devtools-profiler-local/SKILL.mdpackages/devtools_profiler_cli/lib/src/presentation/preparation.dartpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_*/**/*.dart : Consider documenting private helpers when they encode profiler behavior, artifact contracts, protocol semantics, or VM-service assumptions
Applied to files:
packages/devtools_profiler_cli/test/mcp_server_test.dartpackages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/README.mdpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_cli/README.mdpackages/devtools_profiler_core/lib/src/capture/profile_run_request.dartpackages/devtools_profiler_cli/lib/src/presentation/preparation.dartpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_cli/lib/**/command*.dart : Split large CLI commands into focused files instead of growing a monolithic command file
Applied to files:
packages/devtools_profiler_cli/lib/src/presentation/cli_command.dartpackages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_cli/CHANGELOG.mdpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartREADME.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_cli/README.mdskills/devtools-profiler-local/SKILL.mdpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_core/pubspec.yaml : `devtools_profiler_core` may depend on `vm_service`, `dtd`, and `devtools_shared`
Applied to files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_**/pubspec.yaml : Use hosted `devtools_shared` for shared VM and memory models; do not vendor package trees into this workspace
Applied to files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_vm_hookup.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/devtools_profiler_**/pubspec.yaml : Do not add Flutter UI, web UI, or browser-only runtime dependencies to the profiler packages
Applied to files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartREADME.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_cli/README.mdskills/devtools-profiler-local/SKILL.mdpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: `devtools_region_profiler` should stay small and safe to add to Dart or Flutter applications being profiled
Applied to files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartREADME.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/pubspec.yamlpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_core/lib/src/capture/profile_run_result.dartpackages/devtools_profiler_cli/README.mdskills/devtools-profiler-local/SKILL.mdpackages/devtools_profiler_cli/lib/src/presentation/preparation.dartpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: For Flutter targets, remember that release mode, AOT builds, and browser/web targets do not expose the Dart VM service needed by this profiler
Applied to files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/README.mdpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartREADME.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_cli/README.mdskills/devtools-profiler-local/SKILL.mdpackages/devtools_profiler_core/CHANGELOG.mdpackages/devtools_profiler_core/lib/src/capture/runner/profile_session_snapshot_capture.dartpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/lib/src/capture/profile_runner.dartpackages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Run formatting, analysis, and tests before handing work back using: dart format ., dart analyze ., and dart test on all profiler packages
Applied to files:
packages/devtools_profiler_core/pubspec.yamlpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupting_profiler.dartREADME.mdpackages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartpackages/devtools_profiler_cli/pubspec.yamlpackages/devtools_profiler_core/lib/src/capture/profile_run_result.dartpackages/devtools_profiler_cli/README.mdpackages/devtools_profiler_core/test/profile_runner_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/artisanal_widget_app.dartpackages/devtools_profiler_cli/test/cli_test.dartpackages/devtools_profiler_core/test/fixtures/profiled_app/bin/interrupt_parent.dart
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to packages/*/README.md : Package READMEs should explain how that package is used and how it fits into the profiler system. Prefer examples that agents can execute directly from the CLI
Applied to files:
packages/devtools_profiler_core/README.mdREADME.mdpackages/devtools_profiler_cli/README.md
📚 Learning: 2026-04-26T17:03:32.425Z
Learnt from: CR
Repo: kingwill101/devtools-profiler PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-04-26T17:03:32.425Z
Learning: Applies to **/*.dart : Prefer multi-line strings over string concatenation for large text blocks, command output fixtures, JSON examples, and terminal snapshots
Applied to files:
packages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dartskills/devtools-profiler-local/SKILL.mdpackages/devtools_profiler_cli/test/cli_test.dart
🔇 Additional comments (18)
packages/devtools_profiler_cli/lib/src/presentation/cli_command.dart (1)
21-22: Correctly preserves terminal-mode replay behavior.Adding
--terminalonly for inherited-stdio sessions keeps the generated
reproduction command faithful to the original run.packages/devtools_profiler_cli/lib/src/presentation/preparation.dart (1)
63-64: Good metadata propagation in prepared session output.Keeping
terminatedByProfilerandprocessIoModepreserves session semantics
for downstream rendering and CLI command reconstruction.packages/devtools_profiler_core/lib/src/capture/runner/profile_session_controller.dart (1)
139-175: Result builder update is correctly wired for process I/O mode.Requiring
processIoModeat build time and persisting it in the result makes
the capture contract explicit and avoids accidental defaulting.README.md (2)
72-85: Terminal profiling guidance is clear and actionable.The new example and constraints communicate the intended TUI workflow well.
618-620: CLI reference update is consistent with runtime behavior.Documenting
--terminaland its--jsonincompatibility aligns with command
validation and user expectations.packages/devtools_profiler_cli/lib/src/cli/commands/capture_commands.dart (3)
44-49:--terminalflag exposure is well integrated.Option registration and usage examples make the new launch mode discoverable.
Also applies to: 68-68
84-90: Good guardrail for incompatible output modes.The explicit
--terminal+--jsonrejection prevents ambiguous runtime
behavior.
97-100: Request wiring for terminal mode is correct.Mapping
terminalModetoProfileProcessIoMode.inheritStdio(else pipe) keeps
launch semantics deterministic.packages/devtools_profiler_core/README.md (1)
102-119: Core README addition cleanly explains host-level terminal usage.The sample and surrounding notes accurately describe when to use inherited stdio
and interrupt handling.packages/devtools_profiler_cli/CHANGELOG.md (1)
3-12: Changelog entry is aligned with the shipped CLI behavior.The 0.3.0-wip notes accurately summarize the terminal-mode and interrupt-flow
changes.packages/devtools_profiler_core/CHANGELOG.md (1)
3-13: Core changelog update is accurate and complete for this feature set.The entry reflects the key runtime and lifecycle improvements introduced here.
packages/devtools_profiler_core/lib/src/capture/runner/process_launch.dart (7)
11-23: LGTM!Making the stream subscriptions optional is the right approach for inherited-stdio mode where stdout/stderr aren't piped. The relevant code snippet from
profile_runner.dartconfirms callers already handle nullability with?.cancel().
26-38: LGTM!The
expectedVmServiceUrifield with its documentation cleanly captures the deterministic attachment concept.
48-125: LGTM!The conditional logic cleanly separates the two VM-service discovery paths: deterministic port probing for inherited-stdio mode vs. stdout/stderr scraping for pipe mode. The process exit handlers correctly complete the completer with an error if the URI isn't discovered.
381-425: LGTM!The Flutter terminal mode functions correctly restrict
--terminaltoflutter runonly, with clear error messaging explaining whyflutter testisn't supported (no predictable VM-service URI). The port validation logic properly handles both missing and invalid port values.
479-497: LGTM!The helper correctly parses both
--option valueand--option=valueforms. The-prefix check on line 486 technically excludes negative numbers, but this is acceptable since port numbers must be positive.
550-565: LGTM!The connectivity check is clean with proper resource cleanup in the
finallyblock. The 100ms timeout is reasonable for local loopback probing.
344-379: LGTM!The profiler arguments correctly handle terminal mode by:
- Using a fixed port for deterministic attachment
- Disabling service auth codes (necessary since we can't scrape them from output in inherited-stdio mode)
The auth code disabling is appropriate for local profiling scenarios where the VM service binds to loopback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13b676fbd1
ℹ️ 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".
Summary
run --terminaland inherited-stdio launch mode for TUI and alternate-screen Dart/Flutter targets.0.3.0-wipchangelog/version entries for the changed packages.Tests
dart analyze packages/devtools_profiler_protocol packages/devtools_region_profiler packages/devtools_profiler_core packages/devtools_profiler_clidart test packages/devtools_profiler_protocoldart test packages/devtools_region_profilerdart test packages/devtools_profiler_clidart test packages/devtools_profiler_coregit diff --checkdart run packages/devtools_profiler_cli/bin/devtools_profiler.dart run --terminal --cwd packages/devtools_profiler_core/test/fixtures/profiled_app -- dart run bin/artisanal_widget_app.dartSummary by CodeRabbit
New Features
--terminalflag to profile terminal UI (TUI) applications with direct stdin/stdout/stderr accessDocumentation
--terminalconstraints and incompatibility with--jsonoutput