[code] fix gp cli while running next to jb product

[akosyakov]

Since #8025 gp open/preview always exepct JB product if it is enabled, so they break in VS Code Web. We should redesign it by relying on notification service on supervisor instead of env vars.

[andreafalzetti]

after a chat with @akosyakov we concluded that we could start working on this feature incrementally, by adding initially only the endpoint in supervisor and testing it internally by supervisor CLI.

Then, we could have a follow-up ticket to integrate with gp-cli and so on.

[andreafalzetti]

@mustard-mh and I started working on this, we pair programmed for a couple of hours today and made some progress. Besides adding two new endpoints to supervisor-api we have discussed the integration with gp cli.

We've considered 2 approaches, when users use gp open or gp preview:
A. gp cli calls supervisor which is now aware of the latest active IDE/client and supervisor executes the IDE's binary directly.
B. gp cli calls supervisor to retrieve the latest active IDE/client, and gp cli executes the IDE's binary.

The biggest downside of approach A is that we need to implement in supervisor 2 endpoints, one for open one for preview mapping all parameters and flags of those 2 commands. For example gp open has a --wait flag which we should make sure to pass to supervisor. We would stricly couple gp-cli commands and supevisor API.

Another small downside for approach A, is that potentially makes harder returning any error to the user. If the open and preview logic is executed from supervisor, we would need to pro-actively make sure we can report back to the user, in case of failure. Instead, if the logic lives in gp cli the error would be returned directly to the user.

[mustard-mh]

What if we do not use <ide_identity> like vscode jetbrains_gateway etc enum, just use <editor_cli_location> like /ide-desktop/bin/idea-cli so that custom editor can be used here (eg. Users open workspaces without choose any IDE, but use vim with ssh)

And provide cmd like gp set-editor <editor_cli_location>

And we should also provide How to Use Custom IDE (via ssh) section in our doc and #6707 #7570

[akosyakov]

I think the upside of A that we have clean interface and hide implementation details of working with concrete IDEs behind them instead of relying on other ways. B does not sound so clean, since there will be one explicit protocol via interface and another implicit via binaries.

[akosyakov]

The biggest downside of approach A is that we need to implement in supervisor 2 endpoints, one for open one for preview mapping all parameters and flags of those 2 commands.

We should not do it. We should only map that is required for gp cli, i.e. open request should accept absolute path and preview should accept URL. That's it. If someone wants to use a specific capabilities of code CLI, then they should use it directly.

[mustard-mh]

If we exec in supervisor we cannot access to env var of current terminal session

exec in gp-cli it will behavior like user run it directly in this terminal

cc @akosyakov

[akosyakov]

If we exec in supervisor we cannot access to env var of current terminal session

Why do we need to access them? gp cli should resolve the absolute path and url and pass them to JB backend or VS Code windows via supervisor, neither of them are interested in env var of current terminal session.

[mustard-mh]

We don't need to access them currently😂

[mustard-mh]

If we only pass file_loc and url to supervisor, it means that we cannot set GIT_EDITOR=gp open --wait, git cmd like rebase will not be available if we switch IDE

--wait is available for both code and JetBrains IDE

So if we decide to exec in supervisor we can choose

1. parse all args to supervisor as strings..
2. parse all args as struct if needed
3. forget about GIT_EDITOR

cc @akosyakov

[akosyakov]

Right, for open request we should also have wait property.

Are you worrying about backward compatibility to code? gp open does not work like an alias for code or idea-cli but rathe is using them right now as an implementation details. If someone abusing it they should rather to switch using code or idea-cli directly. It will express assumptions more clearly.

[mustard-mh]

I still prefer exec in gp cli, run in supervisor means we need to provide a func like func Exec(argv0 string, argv []string, envv []string) error

open need --wait
preview need --preview

$ gitpod /workspace $ env | grep GP_PREVIEW_BROWSER
GP_PREVIEW_BROWSER=/ide/bin/remote-cli/gitpod-code --preview

So why not ask supervisor for what ide user using now (or the cli location #8191 (comment)), and exec the cli in gp cli

we also parse env before

gitpod/components/gitpod-cli/cmd/preview.go

Lines 63 to 66 in 9619985

	err = unix.Exec(pcmd, append(pargs, url), os.Environ())
	if err != nil {
	log.Fatal(err)
	}

gitpod/components/gitpod-cli/cmd/open.go

Lines 57 to 60 in c8526ae

	err = unix.Exec(pcmd, append(pargs, args...), os.Environ())
	if err != nil {
	log.Fatal(err)
	}

[akosyakov]

I still prefer exec in gp cli, run in supervisor means we need to provide a func like func Exec(argv0 string, argv []string, envv []string) error

We should not run anything in supervisor. It works following:

• VS Code Window subscribes to supervisor to handle notifications
• VS Code Window gets a focus and mark itself as an active client on supervisor
• gp sends open notify request to supervisor
• supervisor detects VS Code Window as an active client
• supervisor propagates open notify request to VS Code Window
• VS Code Window handles the notify request internally and responds when it is done
• supervisor returns to gp
• gp finishes
• VS Code Window loses a focus and mark itself as an inactive client on supervisor

There is no anymore env vars or binaries involved, everything is done via clearly defined protocol.

[mustard-mh]

Do you mean we need to exec in code and gateway by code extension and gateway plugin?

gp cli <-> supervisor <-> code / jetbrains

it will be complex, and not support for custom ide

[akosyakov]

Do you mean we need to exec in code and gateway by code extension and gateway plugin?

We don't need to exec anything. VS Code extenison and JB backend plugin already have enough access to internals to do the proper thing.

it will be complex, and not support for custom ide

It will be easier since there is not undocumented assumptions just one interface to implement for custom IDEs.

[andreafalzetti]

We should not run anything in supervisor. It works following:

* VS Code Window subscribes to supervisor to handle notifications

* VS Code Window gets a focus and mark itself as an active client on supervisor

* gp sends open notify request to supervisor

* supervisor detects VS Code Window as an active client

* supervisor propagates open notify request to VS Code Window

* VS Code Window handles the notify request internally and responds when it is done

* supervisor returns to gp

* gp finishes

* VS Code Window loses a focus and mark itself as an **inactive** client on supervisor

This approach makes sense to me, thanks @akosyakov

Regarding the new endpoints open and preview in supervisor - does it make sense to you to have them in control.proto instead of notification.proto?

Update: actually, it makes more sense to have those in notification considering that they will cause a notification to be sent

[mustard-mh]

Proto approach

• In SetActiveClient we should do (behavior like Subscript)
  • check currentActiveClient if prev value is nil, if not, we need to release it first (de-active)
  • make response to channel, so that we can call vscode jetbrains to do open and preview
  • select (wait) until channel done
• In OpenFile we should do
  • ask currentActiveClient(the channel) to tell the client to do open
• In PreviewLink we should do
  • ask currentActiveClient(the channel) to tell the client to do preview link
• In Action we should
  • ask currentActiveClient(the channel) to tell the client to do open and preview

[akosyakov]

Regarding the new endpoints open and preview in supervisor - does it make sense to you to have them in control.proto instead of notification.proto?
Update: actually, it makes more sense to have those in notification considering that they will cause a notification to be sent

Yes, an idea was to extend existing APIs with new kind of notifications.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[loujaybee]

Not sure if related (or fix in a separate ticket). But a comment from Discord:

Has anyone gotten idea-cli open to work? Git uses $GIT_EDITOR env to open a file when you do rebase -i or commit --amend etc. This is set to: /ide-desktop/bin/idea-cli open --wait on gitpod. I get an IDE notification that a file is supposed to be open, but nothing actually happens. Calling idea-cli open manually from the CLI also does not seem to work.