Add --new-command-timeout to set appium:newCommandTimeout

[JSap0914]

What

Adds a --new-command-timeout <seconds> flag (env MAESTRO_NEW_COMMAND_TIMEOUT) that sets appium:newCommandTimeout on the Appium session when it isn't already set in --caps. An explicit appium:newCommandTimeout in the caps file stays authoritative and is never overridden.

Why

Issue #124 lists two acceptable fixes: (a) honor appium:newCommandTimeout from --caps, or (b) provide a flag/env var to set it.

• (a) already holds on main. createAppiumDriver starts from the caps file and never rewrites appium:newCommandTimeout, so a caps value like 300 reaches the createSession request unchanged. The added TestCreateAppiumDriver_CapsNewCommandTimeoutPreserved locks this in.
• (b) was missing. --help exposes no --new-command-timeout, so a user who can't or doesn't want to edit the caps file (e.g. cloud-generated caps) had no way to raise it. With --parallel N, sessions are created serially before any flow runs, so the earliest sessions idle during pre-creation and get reaped at the server default — the first flow then fails with invalid session id.

This PR adds (b). It is off by default (0 = leave unset) and only fills in appium:newCommandTimeout when neither appium:newCommandTimeout nor a bare newCommandTimeout is present in the caps.

How

• New --new-command-timeout IntFlag (env MAESTRO_NEW_COMMAND_TIMEOUT) in GlobalFlags.
• RunConfig.NewCommandTimeout, sourced from the flag.
• createAppiumDriver injects appium:newCommandTimeout only when it is absent from --caps.
• CHANGELOG.md entry under Unreleased.

Tests

go test ./pkg/cli/ -run NewCommandTimeout:

• TestGlobalFlags_NewCommandTimeoutFlag — flag + env var registered.
• TestCreateAppiumDriver_NewCommandTimeoutInjectedWhenAbsent — flag injects 300 when caps omit it.
• TestCreateAppiumDriver_CapsNewCommandTimeoutPreserved — caps 300 is preserved even with the flag set to 90 (an explicit value always wins).
• TestCreateAppiumDriver_NewCommandTimeoutUnsetByDefault — no flag + no caps value means nothing is added.

ok  github.com/devicelab-dev/maestro-runner/pkg/cli        (NewCommandTimeout)
ok  github.com/devicelab-dev/maestro-runner/pkg/driver/appium   21.0s

gofmt -l and go vet ./pkg/cli/... are clean on the touched files.

Fixes #124

[omnarayan]

Thanks @JSap0914 — this is a careful, well-built PR: the diagnosis is exactly right (option (a) already holds on main, option (b) was the gap), the "explicit caps value always wins" precedence is the correct default, and captureAppiumSessionCaps is a genuinely nice test harness — asserting on the actual alwaysMatch in the outgoing /session request is a better regression guard for the verbatim-forwarding behavior than anything we had.

That said, I'm going to pass on the flag itself, and I want to lay out the reasoning transparently since you put real thought in:

1. The capability already has a home. newCommandTimeout is a standard Appium capability, and --caps is where Appium capabilities belong. Anyone who can pass --new-command-timeout 300 can write "appium:newCommandTimeout": 300 in the --caps file they already supply — and as your own PR confirms, that path works today. A second way to set the same thing mostly adds --help surface without adding capability, and it sets a precedent (a flag per Appium cap) we'd rather not open.

2. The reap is already fixed, without a flag. v1.1.20 addresses [BUG] - maestro-runner ignores the value and forces appium:newCommandTimeout: 90 #124 by keeping pre-created --parallel sessions warm during the serial creation phase (a keepalive ping that resets newCommandTimeout), so the first session no longer idles long enough to be reaped — no user action required, and it works regardless of what the server's timeout is set to.

3. On the farm that opened [BUG] - maestro-runner ignores the value and forces appium:newCommandTimeout: 90 #124, the flag wouldn't have helped. The 90 @devrchoi saw is SauceLabs RDC's own ceiling, applied downstream — it rewrote their caps 300 → 90. So --new-command-timeout 300 would get capped to 90 on Sauce just the same; the keepalive is what actually saves those sessions there. The flag would help on self-hosted Appium or farms that honor the requested value, but that's exactly the case where editing --caps is already easy.

So the net is: the flag is redundant with --caps, and the underlying bug is already resolved a different way.

I'd love to keep your verbatim-forwarding test as a standalone regression guard (with credit to you) — would you be up for a trimmed PR that keeps just captureAppiumSessionCaps + the "caps value preserved" assertion, minus the flag? Happy to help rebase it onto post-1.1.20 main.

Really appreciate the contribution and the clear write-up — please keep them coming.