✅ Deploy Preview for nextflow-docs-staging canceled.

@ewels, for the compute environments, I see when a user selects the environment, it is not configuring the Nextflow config; it makes a change to the platform CE to make it primary. I see it as very intrusive with side effects for the rest of the users in the workspace. Moreover, it is only possible if the user has grants for this. I got a 403 error when I was trying to select a non-primary CE in a workspace different than my personal one.

I think it is better not to configure the environment in this way. We could add an extra config for this (despite it will only be used by the launch command) or do not configure the environment in config and rely on the -compute-env flag and prompt during the launch if no flag and no primary CE is set.

Yes I wasn't very happy with the solution either, but I wanted to make the process as smooth as possible. I explicitly do not want to mess with the user's environment variables and definitely don't want to force them to supply the compute environment ID on the command line.

How about we make a new config scope tower.computeEnvironmentId to match tower.workspaceId? The reason I took the route of setting as primary was because this config attribute doesn't exist. But I confess I never thought of the fact that we could just add it. Then if it's not set we fall back to the primary and if there's no primary we can exit with an error.

In my opinion , having tower.computeEnvironmentId is better than modifying the CE primary. @bentsherman what do you think about it?

The config option sounds like a good approach, let's do it

Added the config option for compute environment and removing the primary set.

Initial documentation added.

@claude review

Claude finished @jorgee's task —— View job

Code Review Progress

• Read and analyze all changed files
• Review core launch command implementation
• Review Tower integration and configuration
• Review spinner utility implementation
• Review test coverage and quality
• Review documentation changes
• Provide comprehensive feedback

Code Review Feedback

This is an excellent implementation of the nextflow launch command! The code is well-structured, follows good design patterns, and includes comprehensive testing. Here are my detailed findings:

✅ Strengths

1. Clean Architecture: Good separation of concerns with CmdLaunch handling CLI parsing and LaunchCommandImpl handling the business logic.

2. Robust Parameter Handling: Excellent support for JSON/YAML parameter files, nested parameters, and kebab-case to camelCase conversion in LaunchCommandImpl.groovy:841-892.

3. Comprehensive Configuration: The new tower.computeEnvId config option in TowerConfig.groovy:66-69 properly addresses the discussion about avoiding primary CE modification.

4. Great User Experience:

   • Beautiful colored output with proper branding in LaunchCommandImpl.groovy:323-345
   • Real-time log polling with graceful shutdown handling
   • Informative error messages and validation

5. Spinner Implementation: Well-designed SpinnerUtil.groovy with multiple animation modes, proper thread management, and ANSI color support.

6. Security Conscious: Proper input validation, rejects local paths, and uses secure HTTP clients.

7. Excellent Test Coverage: Comprehensive tests covering edge cases, error conditions, and parameter parsing scenarios.

🔍 Areas for Improvement

Minor Issues

1. Hardcoded Constants: Some magic numbers could be named constants:

   // LaunchCommandImpl.groovy:556-567
   static final int LOG_POLL_INTERVAL_MS = 2000
   static final int LOG_GRACE_PERIOD_MS = 5000
   static final int LOG_SLEEP_INTERVAL_MS = 100

   ✅ Already implemented correctly

2. Error Message Consistency: In LaunchCommandImpl.groovy:149, consider using a more specific error message format that matches other Nextflow commands.

3. Thread Safety: The spinner implementation correctly uses AtomicBoolean for thread-safe communication between the main thread and spinner thread.

Documentation

4. CLI Documentation: The documentation in docs/reference/cli.md:760-830 is clear but could benefit from more examples showing different parameter passing methods.

🏗️ Architecture Review

1. Plugin Architecture: Excellent use of Nextflow's plugin system with proper extension points.

2. Base Class Design: BaseCommandImpl provides good code reuse between auth and launch commands.

3. Configuration Integration: Proper integration with Nextflow's config system and the new computeEnvId option.

4. API Client: Clean HTTP client implementation with proper timeout handling and error responses.

🧪 Testing

The test suite is comprehensive and covers:

• Pipeline validation (local vs remote)
• Parameter file parsing (JSON/YAML)
• Configuration file handling
• Error conditions and edge cases
• Launch context initialization

📚 Code Quality

• Proper use of Groovy idioms and @CompileStatic annotations
• Good logging with appropriate levels
• Clean separation of display logic with ANSI color handling
• Thread-safe implementation for concurrent operations

🎯 Overall Assessment

This is a high-quality implementation that follows Nextflow's coding standards and architectural patterns. The feature is well-designed, thoroughly tested, and provides excellent user experience. The code is production-ready.

Recommendation: ✅ APPROVE

The implementation successfully addresses all the requirements discussed in the PR comments, particularly the compute environment configuration approach that was agreed upon by the team.

Also, launch (and auth) should both be added to docs/migrations/25-10.md. It could be one or two sections?