[qiffang]

Summary

Add ti exec [flags] [--] command [arg...] to run one command in a remote sandbox with the selected Filesystem at /workspace. Stdout and stderr stream unchanged, the remote exit status is preserved, and executions are never retried. This follows the merged Drive9 #1022 contract at ce0bb29bf1ad24e0e8e1e0107c138061fdea42b6; remote stdin and interactive terminals are not supported.

• Forward terminal and parent-only interruptions exactly once. The first SIGINT/SIGTERM requests cancellation and drain; a second interruption lets the companion detach. Programmatic context cancellation also reaches the companion.
• Keep capability checks local and credential-free, with a disposable home, no stdin or visible output, and a ten-second deadline. Distinguish cancellation, timeout, startup failure, and incompatibility.
• Keep the exact bare ti exec help alias local while allowing ti exec -w repo help, ti exec -- help, and remote --help arguments. Derive metadata inspection from registered flags so flag values and remote arguments cannot enable debug or alter telemetry eligibility.
• Explain Filesystem selection, persistence, working directories, cancellation, and overlapping exit statuses consistently in help, README, and English docs. Remove unused help wrappers and separate implementation verification from the pending release gate.

Installers and the updater reject incompatible companions before replacing installed binaries. Runtime arguments, credentials, environment values, paths, and stream contents remain excluded from debug output, operation logs, and telemetry.

English docs: qiffang/docs@a8e00c35, pinned by the docs submodule.

Validation

Passed on the updated source:

• go test ./...
• make e2e using the compiled CLI and controlled companion
• go vet ./...
• go test -race ./internal/runtimeexec ./internal/cli ./internal/fswrap ./internal/interrupt ./internal/update
• Linux amd64 and Windows amd64 cross-builds
• Targeted English docs Markdown lint and link checks
• git diff --check

The compiled CLI tests cover terminal-group SIGINT, parent-only SIGINT, parent-only SIGTERM, deliberate second interruption, and cancellation during a slow capability check. Unit tests cover programmatic cancellation, capability timeout, disposable state, untouched input, and every registered exec value flag.

Release gate

Local tests establish wrapper behavior, not live Runtime compatibility. Before release, record the distributed companion's exact version, full Git hash, and artifact checksums, then pass make live-e2e-runtime and the complete make live-e2e suite against a Runtime-enabled Filesystem. Real timeout, cancellation/drain, and interrupted-response behavior require live validation.

That release gate has not been executed here. Spec 0036 remains in docs/spec/ until its release evidence is recorded. Runtime remains part of the aggregate live suite; skipped tests do not count as release evidence. Native Windows signal behavior and release packaging were not exercised by these local checks.

[pingcap-cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

qiffang seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[mornyx]

Review summary

This PR adds a top-level ti exec that streams one Drive9 Runtime command, with a new internal/runtimeexec adapter, direct stdin/stdout/stderr wiring in internal/fswrap, a narrow silent-exit marker in internal/apperr, a credential-free companion capability gate, and matching installer/updater checks. The security boundary is genuinely enforced by construction: the probe runs before profile/credential resolution, carries no endpoint or token, and the new tests prove that ordering, separate streams, exact status passthrough, a single attempt, and log/telemetry hygiene. The remaining work is at the edges — interrupt semantics when a signal does not reach the companion, probe side effects, a help routing trap, the duplicated argv pre-scan that now decides shared behavior, and release-gate bookkeeping. Nothing here is a verified merge blocker, but the items below should be decided or fixed.

What looks good

• The Preflight / ReadyService split makes "capability check before credentials" structural rather than conventional, and the tests assert that the gate receives no server, region, FS token, or TiDB Cloud key.
• RunStreaming wires the three streams directly, buffers nothing, and preserves the remote status through a narrowly scoped apperr.SilentExit instead of adding a ti [ERROR] line.
• The noun-first/short-flag exception is deliberately small and stays consistent across README, docs/priciples.md, AGENTS, and both specs; installers and the updater share the same exit-status capability gate.

---

Should fix (non-blocking, strong)

1. Interrupt handling is only correct when the terminal signals the whole process group

• During streaming, RunStreaming (internal/fswrap/runner.go:117) checks ctx once before cmd.Start and then ignores it. If the signal reaches only ti (CI cancel, timeout -s INT, a supervisor, kill -INT <ti-pid>), signal.NotifyContext swallows it and ti blocks until the companion finishes; a second signal does nothing. Measured with a companion that runs 3s: after SIGINT sent only to ti, the full run completed (exit 0) instead of canceling.
• During the gate, cancellation does reach the child because Runner.Run uses exec.CommandContext, but the failure is wrapped as fs.companion_failed (internal/fswrap/runner.go:106), so the errors.Is(err, context.Canceled) guard (internal/runtimeexec/service.go:121) never matches. SIGINT during a slow exec --help exits 1 with "the installed ti-drive9 companion does not support ti exec" instead of an interruption.
• The probe also has no deadline, so a hung exec --help hangs ti exec before any output. The updater bounds the same gate to 10s (internal/update/update.go:780).

Suggestion: check ctx.Err() (not the wrapped cause) after a gate failure and return the interruption; bound the probe; and decide the non-terminal semantics explicitly (bounded grace period, documented detach, or a stated "terminal process-group signals only" contract) with a test for the chosen behavior.

2. The capability probe is not side-effect free

A failing ti exec with no File System selection still creates ~/.ti/drive9-home/default/runtime-capability/.drive9 under a synthetic profile/resource path that no other failing ti fs command creates. The probe also wires the caller's stdin into the child (internal/fswrap/runner.go:78), so a piped invocation can have bytes consumed by exec --help before the real command starts. Suggest nil stdin and no resource-scoped HOME for the probe (temporary home or a skip-home option), plus a test that piped stdin arrives intact at the remote command.

3. ti exec <flags> help cannot run a remote command named help

installHelpCommands gives every command a help child, including exec, and Cobra resolves subcommands before parsing flags. Verified: ti exec -w repo help exits 2 with invalid flag for ti exec help: unknown shorthand flag: 'w' in -w. Bare ti exec help showing ti help is intended, but the flag-bearing form is a normal remote invocation and fails misleadingly; ti exec -w repo -- help works, but that escape is not documented for command-name collisions. Suggest handling the bare help argument in Args/RunE instead of registering the child (or make the child tolerate the exec flag set), and add coverage for help plus the -- escape.

4. The argv pre-scan now decides shared behavior from duplicated flag tables

invocationCommandIndex and execMetadataArgs (internal/cli/exec.go:104, :128) form a third hand-written scanner beside isUpdateInvocation (internal/cli/root.go:307), with separate copies of the global and exec value-flag sets. Their output now drives short-flag rejection (root.go:432), telemetry exclusion (root.go:205), and debug detection (root.go:340) for the whole CLI. Adding a value flag makes the scan read the following value as the command, silently disabling the ti exec bypass, remote---help pruning, and debug detection. Exec identity is also re-derived from cmd.Name() == "exec" in four places (commandEvent, HasNonExecShorthand, usageCommandLine, usageFlagVisible). Suggest one helper built from the Cobra/pflag flag sets that truncates at the first positional and is reused by all three scans, one annotated "is exec" predicate, and a root test that every value-taking flag is understood by the scanner.

5. The release gate is asserted before its dependency exists

make live-e2e now exports TI_LIVE_RUNTIME=1 unconditionally (Makefile:58), so the aggregate suite fails at TestLiveRuntimeExec until an official companion containing Drive9 #1022 is published and Runtime is enabled for the live File System — which also red-lines unrelated db/fs live validation in the meantime. Separately, spec 0036 is filed under docs/spec/done/ while the PR states the live gate has not run; AGENTS reserves done/ for fully implemented and verified requirements, and the spec's own delivery section requires the live pass. Suggest keeping 0036 in docs/spec/ (or marking it release-blocked) until the live gate passes, and making the aggregate-suite coupling an explicit decision — a separate release target, or at minimum a failure message that names the unpublished companion dependency.

Nits

• AGENTS.md: ti exec was added to the "Implemented" list (line 91) but not to "Implemented command behavior" (line 521) or "Registered command surface" (line 675).
• internal/cli/root.go: usageFlagLines (line 922) and formattedFlagUsages (line 986) are now dead wrappers, and the latter is still registered as a Cobra template function (line 36); removing them avoids a future caller bypassing the --output/--query visibility rule.
• internal/cli/exec.go:32: the help text says "ti uploads pending changes", but the ownership boundary says Drive9 drains the mount; align the wording with README/principles.
• e2e/live_test.go:209: byte-exact stderr equality will fail if the real companion ever emits an informational line, and the live test only covers exit status 17, leaving 124/130/255 fake-companion-only.
• README.md:198: remote exits 2/3/4/5 overlap ti's own usage/auth/authz/not-found categories; one sentence telling script authors how to tell them apart would help.

---

Recommendation

Comment. No verified blocking defect; the interrupt contract, probe hygiene, help routing, argv-scan duplication, and release-gate bookkeeping should be decided or fixed before merge.

[qiffang]

Implemented and pushed the fixes in f33d0906.

• Signal forwarding now handles terminal-group SIGINT, parent-only SIGINT/SIGTERM, programmatic cancellation, and deliberate second interruption without duplicating a terminal signal.
• The local capability probe is bounded to ten seconds, uses a disposable HOME, receives no stdin or credentials, and distinguishes interruption, timeout, startup failure, and incompatibility.
• Bare ti exec help remains local; flag-bearing help commands and remote help/debug arguments are preserved. One inspection helper uses the registered flag definitions for metadata and command boundaries.
• Public help, README, AGENTS, the spec, and English documentation now explain explicit selection, persistence, -w, remote stdin limitations, and overlapping exit statuses. Removed unused help wrappers and relaxed only the live stderr assertion that rejected legitimate companion diagnostics.

Validation passed: full unit tests, compiled-binary E2E (including all signal cases and interrupted capability checks), selected race tests, go vet, Linux/Windows cross-builds, and targeted documentation checks. The PR description records the commands and limits.

The source fixes address the findings below. The live release gate remains pending: no claim of live Runtime validation, distributed-companion compatibility, or native Windows signal testing is made. Spec 0036 was moved out of done/, and Runtime remains in the aggregate live release suite.

Original findings, before the fixes

Rechecked this PR at 107fcf42 against the merged Drive9 #1022 contract at ce0bb29b, including the compiled CLI. Following up on the existing review, I would keep the product direction: one ti exec, familiar execution flags, explicit Filesystem selection, unchanged output, and no automatic resubmission. A public CLI should make those few promises dependable.

1. Fix cancellation before merge.

RunStreaming ignores context cancellation after starting the companion. In three runs, sending SIGINT only to the ti PID left both processes waiting; a second SIGINT also did nothing. Sending SIGINT to the process group succeeded in all three controls. Terminal Ctrl-C coverage therefore does not establish the CI/supervisor contract.

Make signal delivery explicit: the first interruption must reach the companion's cancel-and-drain path, and the second must detach deliberately. Cover terminal group signals, signals sent only to ti, and SIGTERM. Avoid blindly forwarding a terminal signal that the child already received: that can turn one Ctrl-C into the companion's second-interrupt detach. Killing the child on the first cancellation is also insufficient because it loses cancellation confirmation.

2. Give command arguments an unambiguous boundary.

I reproduced ti exec -w repo help exiting 2 with unknown shorthand flag: 'w' in -w. The automatically installed help child takes precedence over the remote command.

Keep ti exec help as an exact bare help alias, and ti exec --help as local help. Once execution flags or -- establish a remote invocation, help must be an ordinary command name. ti exec -w repo help, ti exec -- help, and ti exec echo --help should reach the companion unchanged. Remove the competing help child for this command rather than teaching it to swallow execution flags.

Use that same argument boundary for debug detection, logging, and telemetry. A small helper based on registered flag definitions is preferable to several lists of value-taking flags. Test flag values that resemble --help or --debug, as well as remote arguments with those names. This is about preserving arguments and privacy, not introducing another parsing framework.

3. Make the capability check quiet, bounded, and local.

Keep the check before credentials. Give it a short deadline, disconnect stdin, discard help output, and use a disposable isolated home instead of creating a synthetic persistent resource. A missing-selection invocation currently leaves a runtime-capability home behind. I also reproduced SIGINT during a slow check being reported as “the installed ti-drive9 companion does not support ti exec” with exit 1, in three runs. An interruption is not an incompatible installation.

Distinguish cancellation, timeout, failure to launch, and an unsupported command. Normal recovery guidance should use supported ti installation/update steps; TI_DRIVE9_BIN belongs in developer troubleshooting, not the main user journey.

4. Make help explain the job, not the implementation.

A suitable opening is:

Run a command in a remote sandbox.

The selected Filesystem is available at /workspace. Standard output and
standard error stream live. Only files under /workspace persist.

Examples:
  ti exec --file-system-id <id> -w repo npm test
  ti exec -w repo 'npm ci && npm test'
  ti exec -w repo -e CI=1 -t 20m -- make -j8 test

Keep the first example explicit about resource selection; explain that subsequent examples assume TI_FS_FILE_SYSTEM_ID or an explicit FS token. Use “Filesystem directory” in public flag help instead of “Drive9 directory.” Put artifact hashes, companion publication order, and release checks in releasing documentation, rather than between the README's examples and behavior explanation.

Also state the consequential defaults plainly: -w chooses a working directory within the mounted Filesystem, not an isolation boundary; explicit -w does not drain local writes; automatic directory following requires a matching FUSE mount; --fresh replaces the sandbox, not the persisted files. Keep local ti diagnostics actionable without rewriting raw companion/remote stdout or stderr to change branding.

5. Correct two contract assumptions in the earlier discussion.

• Drive9 #1022 has no remote stdin channel in its execution request. Connecting stdin to a local child does not establish remote stdin or interactive-terminal support. Narrow the PR's “live stdin” claim, and do not use a fake companion reading a pipe as proof that printf ... | ti exec cat works remotely. Remote stdin would require an upstream contract change.
• Exact remote exit statuses necessarily overlap ti's own failures, including 2/3/4/5 and 124/130/255. Document that limitation; do not promise that scripts can identify the source from the status alone, invent another exit-code mapping, or require parsing human stderr. Preserve the existing output and no-retry contract.

6. Keep release evidence separate from source completion.

The Drive9 source PR is now merged; that alone does not establish that the downloadable companion is compatible. Record and verify the actual distributed build and checksums, then run the real Runtime gate before releasing ti. Keep Runtime in the complete make live-e2e gate; the existing family-specific targets already let unrelated development proceed independently. Do not make the aggregate pass by silently skipping a shipped feature. Mark implementation completion and pending release validation separately and consistently.

Disposition: resolve the cancellation and command-boundary defects before merging. Address the capability-check behavior and public-contract wording in this change. Internal cleanup and documentation inventory nits do not need separate blocking threads.

Validation: make build and go test ./internal/runtimeexec ./internal/cli ./internal/fswrap passed. The additional checks used the compiled ti binary, isolated homes, and controlled companion processes; they establish local process/argument behavior, not live Daytona compatibility.

[mornyx]

Review summary

Second pass at f33d0906. The round-1 findings are genuinely addressed: interrupts now flow through internal/interrupt and waitStreaming with an isolated companion process group, the capability probe is disposable, stdin-free, and bounded, ti exec <flags> help is routed correctly, the three hand-copied argv scanners are replaced by internal/cli/invocation.go, and the release gate is bookkept honestly (spec back under docs/spec/, releasing docs updated, aggregate coupling made an explicit documented decision). go test ./..., -race on the changed packages, and the runtime signal/exec e2e all pass here.

What remains is at the process boundary: the streaming path still hands ti's stdin to a companion that now runs in its own process group, Windows signal forwarding has two concrete failure modes and no executable coverage, and the capability gate does not pin the binary path it validated. Nothing here is a verified merge blocker for the documented contract, but the first two should be fixed or explicitly decided before this is treated as release-ready.

Resolved since the previous review

• Signal forwarding, second-interrupt detach, and probe interruption are real and covered by compiled-binary tests, including a "duplicate interrupt" assertion.
• The probe is bounded, disposable, credential-free, and no longer touches stdin or creates ~/.ti/drive9-home/....
• ti exec <flags> help runs remotely while the bare form stays local; help wording, AGENTS lists, dead help wrappers, live stderr strictness, and exit-status documentation were all fixed.
• The release gate is no longer claimed as passed: the spec is pending, and docs/releasing.md records the ordering.

What looks good

• internal/interrupt separates "cancel on the first signal" from "forward every signal" cleanly, and context.Cause distinguishes a process signal from programmatic cancellation.
• Runner.Probe has the right shape and tests: temp HOME removed, no stdin, no credentials, ten-second bound, distinct interruption/timeout/incompatibility errors.
• invocation.go derives metadata from registered pflag definitions and centralizes the exec predicate behind an annotation; the tests cover flag values that masquerade as --help/--debug.

---

Should fix (non-blocking, strong)

1. RunStreaming still forwards ti's stdin to a now-background companion

isolateProcess (internal/fswrap/process_unix.go:11) puts the companion in its own process group, but internal/fswrap/runner.go:176 still wires cmd.Stdin = r.Stdin. Verified under a PTY: the companion goes to state T (stopped on SIGTTIN) as soon as it reads stdin; Ctrl-C is then forwarded to a stopped process, and on macOS that produced runtime.outcome_unknown (255) rather than the documented 130, while POSIX keeps such signals pending for a stopped process, so other platforms can wait indefinitely. Without a TTY, piped bytes are silently consumed by the companion, which contradicts the new help/spec statement that remote stdin is unsupported and is the opposite of the probe's no-stdin hardening. Before this commit the child shared ti's foreground group, so this is a round-2 regression. Suggest cmd.Stdin = nil on the streaming path (the upstream protocol has no stdin channel), or an explicit, documented stdin mode plus a PTY test.

2. Windows signal forwarding has two failure modes and no executable coverage

internal/fswrap/process_windows.go:15 always emits CTRL_BREAK_EVENT and returns whatever GenerateConsoleCtrlEvent returns. Two concrete problems: (a) CREATE_NEW_PROCESS_GROUP suppresses console Ctrl+C for the child but not Ctrl+Break, which the console broadcasts to attached processes, so a user pressing Ctrl+Break delivers the event to the companion directly and again through ti — the companion's two-stage protocol can treat the first user interruption as a second one and detach; (b) that Win32 call never returns os.ErrProcessDone, so a late event after the companion exited, or a parent without an attached console, hits the generic error path in waitStreaming (runner.go:215-222), force-kills the companion, and reports 255 instead of letting it drain or preserving a completed status. All new signal tests are //go:build !windows, so none of this is exercised. Suggest treating "target group gone / no console" as benign and waiting for cmd.Wait(), suppressing the duplicate forward if the event cannot be scoped, and adding at least a programmatic-cancellation test on Windows CI.

3. The capability check does not pin the companion it validated

Preflight resolves the companion inside Runner.Probe (internal/runtimeexec/service.go:65), but ReadyService carries only the original empty CompanionPath; Exec rebuilds the runner (service.go:86) and RunStreaming calls locateCompanion again. A concurrent ti update/install (atomic replace) or a symlink swap between the two calls means ti runs a binary that was never capability-checked, defeating the gate's stated guarantee. Suggest having Probe return the resolved path and storing it in ReadyService so execution uses exactly the probed binary.

Nits

• internal/cli/root.go:129-131,191: inspectInvocation runs three times per invocation; after the bare-help rewrite only help changes. Let normalizeExecHelp report the rewrite, or pass the computed invocation into resolvedTelemetryCommand.
• e2e/live_test.go:210-215: the automated live gate still proves only a successful run and exit 17. If 124/130/255 remain manual release evidence, record the exact commands and expected observations in docs/releasing.md (for example ti exec -t 1s 'sleep 60' → 124); otherwise the gate can pass without ever exercising the round-2 cancellation behavior.

---

Recommendation

Comment. No verified blocking defect; the round-1 fixes hold, but the stdin/process-group regression, the untested Windows forwarding semantics, and the probe/exec path mismatch should be decided or fixed before this is treated as release-ready.