refactor(agoric-cli): clarify flow of inputs to outputs in start.js (WIP)

[dckc]

This is very rough; I'm not sure how far I'll pursue it. Early feedback is welcome.

refs: #6687, #2160

Description

While trying to remove sim-behaviors.js from boot.js (#6687), I re-discovered argv.ROLE. In trying to figure out whether we still use it, I struggled to read packages/agoric-cli/src/start.js. This is an attempt to take the one 700+ line function and refactor it to make clear what inputs are used where.

Security Considerations

The result should obey ocap discipline (#2160).

Documentation Considerations

It's not intended to have any user-visible impact.

Testing Considerations

I started adding unit tests to confirm that the refactor preserves all relevant functionality.

[turadg]

+1 to using Command. though with it, this file should go in src/commands. You might want do the Commander refactorings in a separate PR.

[dckc]

hm... using src/commands seems to be the exception, not the rule. main.js looks like:

import cosmosMain from './cosmos.js';
import deployMain from './deploy.js';
import publishMain from './main-publish.js';
import initMain, { makeInitCommand } from './init.js';
import installMain from './install.js';
import setDefaultsMain, { makeSetDefaultsCommand } from './set-defaults.js';
import startMain from './start.js';
import followMain, { createFollowCommand } from './follow.js';
import { makeOpenCommand, makeTUI } from './open.js';
import { makeWalletCommand } from './commands/wallet.js';

[turadg]

Depends on how you look at it. What I am requesting is that files that import the Command class (and export an instance of it) go in src/commands. There are 4 examples of that in master. The one exception in src is src/main.js which is making the program. That's cool.

These others are not command instances:

import cosmosMain from './cosmos.js';
import deployMain from './deploy.js';
import publishMain from './main-publish.js';
import installMain from './install.js';
import startMain from './start.js';

And these in your snippet actually are Commands, but ones you're making in this PR:

// makeInitCommand
// makeSetDefaultsCommand
// createFollowCommand
// makeOpenCommand

So I'm requesting that they go with the other Command objects in src/commands.

[dckc]

I don't plan to export an instance of Command; I plan to export a maker function that takes the necessary authority as args and creates a Command.

[turadg]

Sorry for my impression: exporting a maker function that returns an instance of Command. That's what createFollowCommand is, right? That's also what these are:

agoric-sdk/packages/agoric-cli/src/commands/psm.js

Line 59 in f1ff7b9

export const makePsmCommand = async logger => {

agoric-sdk/packages/agoric-cli/src/commands/perf.js

Line 26 in f1ff7b9

export const makePerfCommand = async logger => {

agoric-sdk/packages/agoric-cli/src/commands/oracle.js

Line 20 in f1ff7b9

export const makeOracleCommand = async logger => {

agoric-sdk/packages/agoric-cli/src/commands/wallet.js

Line 31 in f1ff7b9

export const makeWalletCommand = async () => {

When you wrote "seems to be the exception, not the rule" I took it that you were appealing to precedent. The for functions like you describe (returning a Command instance) is that all four of them are in src/commands. It seems important to you that you don't put those files in src/commands. I don't understand why but I'll wait for R4R before engaging on the topic again in this PR.

[turadg]

ditto, src/commands

[turadg]

yay tests!