Skip to content

feat: add resource usage warnings for heavyweight commands#105

Merged
Miyamura80 merged 4 commits intomasterfrom
feat/resource-warnings
Apr 2, 2026
Merged

feat: add resource usage warnings for heavyweight commands#105
Miyamura80 merged 4 commits intomasterfrom
feat/resource-warnings

Conversation

@Miyamura80
Copy link
Copy Markdown
Contributor

@Miyamura80 Miyamura80 commented Apr 2, 2026

Summary

  • Add static resource usage warnings before Docker container creation (~4 GB memory, 4 CPU cores), Tart VM cloning (~10+ GB disk, 2-VM Apple license limit), test suite runs (aggregate per-test estimates), and init-macos golden image provisioning (~10-20 GB disk)
  • Add --quiet / -q global flag to suppress these warnings
  • New src/warnings.rs module with warning functions that read configured limits from Config

Test plan

  • cargo build passes
  • All 531+ unit tests pass
  • --quiet / -q flag appears in --help output
  • Manual: desktest run task.json shows Docker resource warning
  • Manual: desktest run task.json -q suppresses the warning
  • Manual: desktest suite ./tests shows aggregate suite warning

🤖 Generated with Claude Code


Open with Devin

Miyamura80 and others added 2 commits April 2, 2026 09:52
Warn users about memory, CPU, and disk allocation before creating
Docker containers, Tart VMs, or running test suites. Warnings are
static and informational (no errors). Suppressible via --quiet/-q.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 2, 2026

Greptile Summary

This PR introduces static resource-usage warnings for heavyweight commands (desktest run, desktest suite, desktest interactive, desktest init-macos) and a --quiet / -q global flag to suppress them. A new src/warnings.rs module holds the four warning functions, which read memory/CPU limits from Config to produce accurate per-environment messages. The N+1 suite-warning flood from earlier review cycles has been properly addressed by constructing an inner_run with quiet: true for per-task calls inside run_suite.

Key changes:

  • src/warnings.rs: new module with warn_docker_resources, warn_tart_resources, warn_suite_resources, and warn_init_macos_resources
  • src/orchestration.rs: adds quiet to RunConfig; emits the appropriate warning before session creation
  • src/suite.rs: emits one aggregate suite warning, then suppresses per-task warnings via inner_run
  • src/interactive.rs: emits Docker warning in both interactive sub-modes (mutually exclusive, no double-warning)
  • src/main.rs / src/cli.rs: threads --quiet through all command branches

Issues found:

  • warn_suite_resources uses total (all tests, including native) in its Docker-only and Tart-only message branches, causing an inaccurate count when native tests are mixed in alongside Docker or Tart tests (e.g. "Running 5 test(s) — each Docker test allocates…" when only 3 of 5 tests are Docker).
  • The all-native fallback branch uses a "Warning:" prefix for a path explicitly noted as "lightweight", which may unnecessarily alarm users; "Note:" would be more appropriate.

Confidence Score: 5/5

  • Safe to merge; all findings are P2 cosmetic/informational inaccuracies in warning text with no impact on runtime behaviour.
  • Prior P1 issues (N+1 warning flood, misleading nano-CPU comment) are confirmed resolved. The two remaining comments are both P2: one is a count inaccuracy in a warning message that doesn't affect execution, and one is a word-choice nit ("Warning" vs "Note"). No correctness, data-integrity, or security issues were found.
  • src/warnings.rs — specifically the Docker-only and Tart-only branches of warn_suite_resources (lines 67-84) and the all-native branch (lines 104-110).

Important Files Changed

Filename Overview
src/warnings.rs New module with four warning functions; warn_suite_resources inaccurately uses total (includes native tests) in Docker-only and Tart-only branches when native tests are mixed in, and emits a misleading "Warning:" prefix for the all-native path.
src/orchestration.rs Adds quiet field to RunConfig and emits warn_tart_resources or warn_docker_resources before session creation; correctly skips warnings for native sessions.
src/suite.rs Adds suite-level warn_suite_resources call and correctly creates inner_run with quiet: true to suppress per-test warnings, fixing the previously noted N+1 flood.
src/interactive.rs Adds Docker resource warning in both run_interactive_pause and run_interactive_step; the two paths are mutually exclusive so no double-warning can occur.
src/cli.rs Adds --quiet / -q global CLI flag correctly wired to all command branches.
src/main.rs Propagates quiet: cli.quiet into every RunConfig construction site and adds the init-macos warning before provisioning; no issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    CLI["CLI invocation\n(--quiet flag)"] --> CMD{Command?}

    CMD -- "run / attach / replay" --> ORC["orchestration::run_task"]
    CMD -- "suite" --> SUITE["suite::run_suite"]
    CMD -- "interactive" --> INT["interactive::run_interactive"]
    CMD -- "init-macos" --> INIT["init_macos::run_init_macos"]

    ORC --> Q1{quiet?}
    Q1 -- No, Tart --> WT["warn_tart_resources()"]
    Q1 -- No, Docker --> WD["warn_docker_resources(config)"]
    Q1 -- No, Native --> SKIP1["(skip — no resources)"]
    Q1 -- Yes --> SKIP1

    SUITE --> Q2{quiet?}
    Q2 -- No --> WS["warn_suite_resources(config, apps)"]
    Q2 -- Yes --> IR
    WS --> IR["inner_run { quiet: true }"]
    IR --> ORC

    INT --> Q3{step?}
    Q3 -- No --> PAUSE["run_interactive_pause\nwarn_docker_resources()"]
    Q3 -- Yes --> STEP["run_interactive_step\nwarn_docker_resources()"]

    INIT --> Q4{quiet?}
    Q4 -- No --> WI["warn_init_macos_resources()"]
    Q4 -- Yes --> PROV["Provisioning"]
    WI --> PROV
Loading

Reviews (3): Last reviewed commit: "🐛 Make suite warning platform-aware for..." | Re-trigger Greptile

greptile-apps[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

- Fix N+1 warning flood in suite mode: suppress per-test warnings
  when running inside a suite (suite-level summary is sufficient)
- Fix misleading comment on DEFAULT_CPU_CORES constant

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Miyamura80
Copy link
Copy Markdown
Contributor Author

@greptileai

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

Comment thread src/warnings.rs Outdated
Suite warning now inspects task app types to show accurate resource
info: Docker memory/CPU for Docker tests, disk usage for Tart VMs,
and appropriate messaging for mixed or native-only suites.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Miyamura80
Copy link
Copy Markdown
Contributor Author

@greptileai

@Miyamura80 Miyamura80 merged commit 2b808bd into master Apr 2, 2026
4 checks passed
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