Skip to content

fix: service detection in vip dev-env logs #2807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sjinks merged 2 commits into from
May 8, 2026
Merged

fix: service detection in vip dev-env logs #2807

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea87bd9
fix: service detection in `vip dev-env logs`
sjinks May 1, 2026
d17ec4c
test: update tests
sjinks May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion __tests__/devenv-e2e/011-logs.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe( 'vip dev-env logs', () => {
);
expect( result.rc ).toBeGreaterThan( 0 );
expect( result.stderr ).toContain(
"Error: Service 'foobar' not found. Please choose from one:"
"Error: Service 'foobar' not found. Please choose from:"
);
} );

Expand Down
12 changes: 6 additions & 6 deletions src/lib/dev-environment/dev-environment-core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
LandoExecOptions,
getProxyContainer,
removeProxyCache,
getLandoApplication,
} from './dev-environment-lando';
import { AppEnvironment } from '../../graphqlTypes';
import app from '../api/app';
Expand Down Expand Up @@ -344,15 +345,14 @@ export async function showLogs(

const instancePath = getEnvironmentPath( slug );

debug( 'Instance path for', slug, 'is:', instancePath );
debug( 'Instance path for %s is %s', slug, instancePath );

if ( options.service ) {
const appInfo = await landoInfo( lando, instancePath );
if ( ! appInfo.services.includes( options.service ) ) {
const application = await getLandoApplication( lando, instancePath );
const services = application.info.map( service => service.service );
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

Service names are being derived from application.info here. App also exposes application.services (string[]) which is the canonical service list and is already filtered in initLandoApplication (e.g., initOnly services removed). Using application.services avoids relying on info shape/completeness and keeps the validation consistent with the service list Lando itself tracks.

Suggested change
const services = application.info.map( service => service.service );
const services = application.services;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@sjinks sjinks May 1, 2026

Choose a reason for hiding this comment

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

No, this is wrong. application.services differs from application.info. The former lists only active services (e.g., excluding init-only containers), while the latter includes information about all services.

For example,

  • Services from application.info: ['nginx', 'php', 'database', 'memcached', 'wordpress', 'vip-mu-plugins', 'demo-app-code']
  • Services from application.services: ['nginx', 'php', 'database', 'memcached']

Init-only containers also produce logs, and if we use application.services, we won't be able to watch their logs without the low-level docker logs command

if ( ! services.includes( options.service ) ) {
throw new UserError(
`Service '${
options.service
}' not found. Please choose from one: ${ appInfo.services.toString() }`
`Service '${ options.service }' not found. Please choose from: ${ services.join( ', ' ) }`
);
Comment thread
sjinks marked this conversation as resolved.
Comment thread
sjinks marked this conversation as resolved.
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dev-environment/dev-environment-lando.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ async function landoRecovery( lando: Lando, instancePath: string, error: unknown
}
}

async function getLandoApplication( lando: Lando, instancePath: string ): Promise< App > {
export async function getLandoApplication( lando: Lando, instancePath: string ): Promise< App > {
const started = new Date();
try {
if ( appMap.has( instancePath ) ) {
Expand Down
5 changes: 2 additions & 3 deletions types/lando/plugins/lando-core/lib/utils.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import App from 'lando/lib/app';

export interface AppInfo {
export interface AppInfo extends Record< string, unknown > {
name: string;
location: string;
services: string[];
[ key: string ]: unknown;
services: string;
}

export function getHostPath( mount: any ): any;
Expand Down
Loading