Terminal post-attach commands

[joshaber]

Problem

We want to codify the post-attach commands which clients should run in parallel, interactive terminals.

For example: I always want my server (npm start) and my file watcher (npm run watch) running when I connect.

Proposal

• The client will decide how to run these commands interactively in a manner appropriate for it.
  • For example, rich clients like VS Code and JetBrains could run them as Terminals, command line clients could ignore them, run them in a tmux session, etc.
• The client will ensure the commands are running each time they attach and recreate them if they are not.
  • Because this is client-specific, connecting with multiple clients could result in the commands running multiple times.

Details

1. Modify postAttachCommand to:

   postAttachCommand: string | array | {[key: string}: TerminalPostAttachCommand};
   
   type TerminalPostAttachCommand = string | {command: string | array, terminal: boolean}

2. The object form of postAttachCommand will default to terminal: true.
3. The key is the identifier for the post attach command. This can be referenced by client-specific settings, see below.
4. The value or command is the command to run.

Example:

{
  "postAttachCommand": {
    "server": "npm start", // defaults to `terminal: true`
    "watcher": "npm run watch",
    "greetings": {
      "command": "echo hello",
      "terminal": false // The current headless post attach behavior
    }
  },

  // Client-specific customizations using the keys above:
  "customizations": {
    "vscode": {
      "terminals": {
        "server": { // Key matches the `postAttachCommand` key above
          "icon": "bug"
        },
        "watcher": {
          "name": "All-seeing eye",
          "icon": "eye",
          "color": "red"
        }
      }
    }
  }
}

Open Questions

1. Should terminal: true be the default behavior for postAttachCommand?
   • This would be a breaking change, but looking at postAttachCommand usage in the wild it looks like many of the use cases would either (1) be improved by running in interactive terminals, or (2) surface to the user that postAttachCommand is the wrong lifecycle hook for the behavior they want.

cc @Chuxel @jkeech since this is coming out of conversations with y'all

[Chuxel]

Generally I like where this is going! /cc @craiglpeters as well

interactive: true commands will not be run by devcontainer CLI.

Hmmm. Is this trying to say that this wouldn't be automatically run by the any implementing system at the time it starts, but hold until the attach is complete? If so, that's the definition of waitFor. Also, interactive implies the shell would be run with -i. This seems to be more about allocating a terminal / tty, right? VS Code settings, for example, may or may not default to a -i shell since you can change that behavior.

Perhaps this is more "wait for tty" (the -t in docker run -it). Then, anyone could decide when they've met the requirement for a tty. In the dev container CLI case, I'd argue that if devcontainers/cli#59 was implemented, something like a-t flag on exec could be that signal -- which then would allow command line tools to meet the same needs via the CLI.

Assuming all of this tracks, possible names for the property could be:

• "terminal": true
• "waitForTerminal": true

We could also talk about this in terms of whether it's a "background" executed step or not where "foreground" implies in a terminal. We'd need to figure out what the exec flag would be or an equivalent for the CLI, but that would allow:

• "background": false

[joshaber]

Perhaps this is more "wait for tty" (the -t in docker run -it). Then, anyone could decide when they've met the requirement for a tty.

To confirm: are you saying that it'd be up to the attaching tool to decide whether/how to fulfill the ask for a TTY with which to run the commands? Devcontainer CLI could do it with the -t flag, VS Code could do it with terminals, etc.

If I understand you right that sounds like what we want 👍

• "waitForTerminal": true

This seems to speak more to timing, where I think as a user the significant thing is where/how the command is run.

Foreground/background make intuitive sense to me 🤔

[Chuxel]

To confirm: are you saying that it'd be up to the attaching tool to decide whether/how to fulfill the ask for a TTY with which to run the commands? Devcontainer CLI could do it with the -t flag, VS Code could do it with terminals, etc.

If I understand you right that sounds like what we want 👍

Yep! Exactly.

• "waitForTerminal": true

This seems to speak more to timing, where I think as a user the significant thing is where/how the command is run.

Foreground/background make intuitive sense to me 🤔

Yeah, the other option would just be to embrace tty as the name. That is how -t is described for docker run.

[joshaber]

cc also @ThisIsKirsch for thoughts on nomenclature

[joshaber]

Updated OP with the outcomes 👍

Notes from sync:

• We probably want something like devcontainer exec-post-attach <name> to run commands by name so that variable substitution, etc. happens
• Waiting on @chrmarti's input on whether to change the default postAttachCommand behavior, initial impression is that it makes sense to change
• Use terminal as the configuration key name

[chrmarti]

The examples npm start and npm run watch would also work as postStartCommand (run after starting the container). Would using that make sense? The advantage of postStartCommand is that it won't retrigger when the client only briefly disconnects and the connects again. postAttachCommand would start a second set of npm start and npm run watch commands in that case.

Is the goal of "terminal": true to show the output in the UI (instead of hiding it) or to also allow for user input to the commands? VS Code currently does both for all lifecycle commands (while using the dev containers CLI). I wonder if that should be the recommended behavior without even adding the "terminal" flag. When there is no UI available (e.g., in CI) there would be no way of providing "terminal": true anyway.

The example includes "customizations" for VS Code, are these part of the proposal?

[Chuxel]

The examples npm start and npm run watch would also work as postStartCommand (run after starting the container). Would using that make sense? The advantage of postStartCommand is that it won't retrigger when the client only briefly disconnects and the connects again. postAttachCommand would start a second set of npm start and npm run watch commands in that case.

@chrmarti That's a good point - if you attach the tool multiple times it probably shouldn't fire. Correct @joshaber ?

Is the goal of "terminal": true to show the output in the UI (instead of hiding it) or to also allow for user input to the commands? VS Code currently does both for all lifecycle commands (while using the dev containers CLI). I wonder if that should be the recommended behavior without even adding the "terminal" flag. When there is no UI available (e.g., in CI) there would be no way of providing "terminal": true anyway.

It is both. Yeah honestly, I don't think there's a reason to not pipe stdin as well as stdout for any of the commands. That said, some CLIs do behave differently depending on whether they are in a terminal or not (e.g. using if [ -t 1 ]). So, that's really my only reservation is with changing the default. I also think Codespaces needs a way to hint that the terminal should be shown given it is obscured by default and other implantations could do something similar.

The example includes "customizations" for VS Code, are these part of the proposal

No that's a tool specific example. I believe the thinking here is that this would actually tie to profiles in some way - so there's VS Code considerations, but not directly part of the spec since other products/services will have different needs.

[chrmarti]

Just to clarify: Currently the lifecycle commands all run in a PTY (also when the output is not shown in a terminal), so [ -t 1 ] is always true.

If Codespaces uses the extension API to open the terminal (I assume it will), there is a flag to immediately reveal the terminal.

[Chuxel]

Just to clarify: Currently the lifecycle commands all run in a PTY (also when the output is not shown in a terminal), so [ -t 1 ] is always true.

Ah cool. So then, yeah, I really don't see any reason not to make them all support stdin by default TBH. At that point, is the flag even needed? I think part of this is a hint to put the terminal in focus, but isn't that more related to the tool specific layout config?

What do you think @joshaber ?

[jkeech]

I see a few key parts of this proposal:

1. Clarify that the postAttachCommand should be executed after clients finish attaching (not pre-attach, or concurrent with attachment). The CLI will need to be updated to allow running postAttachCommand independent of other lifecycle hooks, which would then be triggered by clients after they attach. separating postAttachCommand execution in the CLI will enable the ordering to happen correctly and it will also enable clients to stream the contents into a terminal if desired instead of attaching a terminal to an already-running process log stream.
2. Allow specifying multiple named postAttachCommands to enable layout hints for clients to run the commands side-by-side, etc.

If we switch everything to default to TTY with stdin and defer execution until clients request it, then I don't think we need to support the terminal: false in the postAttachCommand. There still might be a layout hint in customizations to hide one of the postAttachCommands if desired by the user, but that's more of a layout/presentation choice and doesn't affect the sequencing or execution of the command.

[joshaber]

I've started formalizing this into a proposal at #88 so that we can nail down the specifics.

[Chuxel]

CLI PR: devcontainers/cli#172

[bamurtaugh]

Closing as this has been merged as a proposal into the spec: https://github.com/devcontainers/spec/blob/main/proposals/parallel-lifecycle-script-execution.md.

Thanks all!