From 107fcf42cb20056af1e56cd7283b83d865cb4b8f Mon Sep 17 00:00:00 2001 From: qiffang Date: Tue, 20 Oct 2026 02:35:55 +0800 Subject: [PATCH 1/2] feat: add Runtime exec command --- AGENTS.md | 22 +- Makefile | 7 +- README.md | 22 +- cmd/ti/main.go | 4 +- docs/pingcap-docs/docs | 2 +- docs/priciples.md | 15 +- docs/releasing.md | 6 +- docs/spec/done/0001-cli-foundation.md | 5 + docs/spec/done/0036-runtime-exec.md | 199 +++++++++++++++ e2e/cli_test.go | 182 +++++++++++++- e2e/installer_test.go | 7 +- e2e/live_test.go | 22 ++ e2e/runtime_signal_unix_test.go | 79 ++++++ e2e/testdata/fake-drive9.go | 32 +++ internal/apperr/apperr.go | 21 ++ internal/cli/exec.go | 153 ++++++++++++ internal/cli/exec_test.go | 60 +++++ internal/cli/root.go | 52 +++- internal/cli/root_test.go | 4 +- internal/fs/drive9_companion_test.go | 2 +- internal/fswrap/runner.go | 52 +++- internal/runtimeexec/service.go | 155 ++++++++++++ internal/runtimeexec/service_test.go | 347 ++++++++++++++++++++++++++ internal/update/update.go | 4 + internal/update/update_test.go | 30 +++ scripts/install.ps1 | 5 + scripts/install.sh | 4 + 27 files changed, 1457 insertions(+), 36 deletions(-) create mode 100644 docs/spec/done/0036-runtime-exec.md create mode 100644 e2e/runtime_signal_unix_test.go create mode 100644 internal/cli/exec.go create mode 100644 internal/cli/exec_test.go create mode 100644 internal/runtimeexec/service.go create mode 100644 internal/runtimeexec/service_test.go diff --git a/AGENTS.md b/AGENTS.md index e6af776..1aff151 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -88,6 +88,7 @@ Implemented: - `ti configure` - `ti update --check` - `ti update` +- `ti exec` - `ti db create-db-cluster` - `ti db list-db-clusters` - `ti db describe-db-cluster` @@ -240,6 +241,7 @@ make live-e2e-fs make live-e2e-fs-git make live-e2e-fs-journal make live-e2e-fs-vault +make live-e2e-runtime make live-e2e make release-snapshot make clean @@ -263,7 +265,7 @@ delivery path before dropping only that temporary database. It must not run as part of `make test`, `make e2e`, or any live-e2e target. The `make live-e2e-` targets build `bin/ti` and run only the selected top-level command family against the `live-e2e` profile by default. Keep -configure, db, fs, fs-git, fs-journal, and fs-vault tests +configure, db, fs, fs-git, fs-journal, fs-vault, and exec tests independently selectable. Do not make a focused family target run tests from a different family, and do not add separate mutating/non-mutating variants. `make live-e2e` runs every live family together in one test process and remains @@ -368,6 +370,7 @@ internal/fs/tokenmgmt/ Filesystem token lifecycle and local rotation safety internal/oplog/ local JSONL operation log writer internal/output/ structured JSON/text/raw rendering internal/query/ JMESPath query application +internal/runtimeexec/ one-shot Drive9 Runtime command adapter internal/secretinput/ no-echo secret input helper internal/settings/ global settings parsing and legacy logging migration internal/telemetry/ CLI eligibility, identity, event, and delivery path @@ -392,12 +395,14 @@ without underscores. Follow these rules unless `docs/priciples.md` is updated: - The command tree is at most two levels: `ti [subcommand]`. -- `ti configure` and `ti update` are the only intentional top-level verb - exceptions. `ti configure` is the only interactive command. +- `ti configure`, `ti update`, and `ti exec` are the only intentional top-level + verb exceptions. `ti exec` stays top-level to match the public Drive9 + Runtime interaction. `ti configure` is the only interactive command. - Other top-level commands are nouns such as `db` and `fs`. -- Use long flags only, for example `--profile` and `--db-cluster-name`. -- Do not add short flags or one-letter aliases. The current CLI rejects short - flags before invoking Cobra. +- Use long flags except for the exact `ti exec` compatibility aliases `-w`, + `-C`, `-e`, `-t`, and `-v`. +- Do not add any other short flags or one-letter aliases. The CLI rejects short + flags outside `ti exec` before invoking Cobra. - `ti fs` Unix-style aliases are command-name aliases only. They must keep the same long flags, output modes, auth, permissions, dry-run behavior, and command handlers as their canonical commands. @@ -1061,11 +1066,12 @@ Generated `ti fs` resource API keys live only in the ID-keyed credentials files under `~/.ti/fs_credentials/`. User-facing docs and commands must call these `ti fs` API keys or resource credentials, never reference implementation API keys. Filesystem data-plane -commands route through the installer-managed Drive9 companion binary named +commands and `ti exec` route through the installer-managed Drive9 companion binary named `ti-drive9`. ti owns profile loading, region resolution, credential storage, preflight errors, output/query handling, and command naming; Drive9 owns the filesystem runtime semantics for data-plane file operations, FUSE/WebDAV mount, -FUSE mount drain, layer behavior, pack/unpack, Git workflows, journal, and vault. +FUSE mount drain, layer behavior, pack/unpack, Git workflows, journal, vault, +and Runtime execution/streaming/cancellation. Do not reintroduce a runtime fallback to ti-native fs behavior. Public fs service methods must route through the Drive9 companion path unconditionally; do not add switches such as `UseDrive9Companion` or hidden environment flags diff --git a/Makefile b/Makefile index 030b89e..a885015 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ ifneq ($(strip $(TELEMETRY_ENDPOINT)),) LDFLAGS += -X $(MODULE)/internal/version.telemetryEndpoint=$(TELEMETRY_ENDPOINT) endif -.PHONY: all build build-telemetry-backend build-telemetry-migrator test e2e telemetry-e2e live-e2e live-e2e-configure live-e2e-db live-e2e-fs live-e2e-fs-git live-e2e-fs-journal live-e2e-fs-vault release-snapshot clean +.PHONY: all build build-telemetry-backend build-telemetry-migrator test e2e telemetry-e2e live-e2e live-e2e-configure live-e2e-db live-e2e-fs live-e2e-fs-git live-e2e-fs-journal live-e2e-fs-vault live-e2e-runtime release-snapshot clean all: build @@ -55,7 +55,7 @@ telemetry-e2e: build build-telemetry-backend build-telemetry-migrator TI_TEST_TELEMETRY_TIDB_DSN="$$TI_TEST_TELEMETRY_TIDB_DSN" TI_E2E_BIN="$(abspath $(TI_BIN))" TI_TELEMETRY_BACKEND_E2E_BIN="$(abspath $(TELEMETRY_BACKEND_BIN))" TI_TELEMETRY_MIGRATOR_E2E_BIN="$(abspath $(TELEMETRY_MIGRATOR_BIN))" TI_TELEMETRY_E2E=1 $(GO) test ./e2e -count=1 -v -run '^TestTelemetryDeliveryToTiDB$$' live-e2e: build - $(LIVE_E2E_RUN) -run '^TestLive' + TI_LIVE_RUNTIME=1 $(LIVE_E2E_RUN) -run '^TestLive' live-e2e-configure: build $(LIVE_E2E_RUN) -run '^TestLive(ProfileConfigured|CLICommandSurface)$$' @@ -75,6 +75,9 @@ live-e2e-fs-journal: build live-e2e-fs-vault: build $(LIVE_E2E_RUN) -run '^TestLiveFSVault' +live-e2e-runtime: build + TI_LIVE_RUNTIME=1 $(LIVE_E2E_RUN) -run '^TestLiveRuntimeExec$$' + release-snapshot: $(GORELEASER) release --snapshot --clean diff --git a/README.md b/README.md index 51ae14d..743428a 100644 --- a/README.md +++ b/README.md @@ -179,6 +179,24 @@ Automatic mounting uses FUSE on Linux and WebDAV on macOS and Windows. macOS use Mount commands start the companion runtime in the background, wait until the mount is ready, and then return a structured result. Use `ti fs unmount-file-system` or `ti fs-vault unmount-vault` to end a mount. The public CLI does not expose a foreground mount mode. +Run a command remotely without creating or cleaning up a sandbox yourself: + +```shell +ti exec -w repo npm test +ti exec -w repo 'npm ci && npm test' +ti exec -w repo -e CI=1 -e GITHUB_TOKEN -t 20m -- make -j8 test +``` + +`ti exec` requires a `ti-drive9` companion that contains the Runtime command +from Drive9 #1022. The installer and updater reject older companions before +installing either binary. A published ti release must record the exact Drive9 +build and checksums and pass the real `make live-e2e-runtime` release gate; +fake-companion tests or a skipped live test are not release evidence. + +`ti exec` mounts the selected Filesystem at `/workspace`, streams stdout and stderr live, and exits with the remote command's status. It uses the same `--file-system-id` / `TI_FS_FILE_SYSTEM_ID`, `--fs-token` / `TI_FS_TOKEN`, profile, and region resolution as other data-plane commands. Inside a matching ti-created FUSE mount, omit `-w`: pending local changes are drained first and the command runs in the corresponding directory. WebDAV mounts are not inferred, so macOS and Windows users normally pass `-w`. + +The Runtime flags intentionally match `drive9 exec`: `-w/--workspace`, `-C/--cwd`, repeatable `-e/--env`, `-t/--timeout`, `--fresh`, `--layer`, `--read-only`, and `-v/--verbose`. Flag parsing stops at the remote command; use `--` when the command itself starts with a dash. `--output`, `--query`, and `--dry-run` are not supported because output is a pair of live byte streams. Timeout exits `124`, Ctrl-C cancellation exits `130`, and an unknown outcome exits `255`. Executions are never retried: after `255`, inspect the workspace before deciding whether to run the command again. + Mount readiness means the mounted path is readable: `ti fs mount-file-system` and `ti fs-vault mount-vault` keep polling the mount path until directory listing succeeds, bounded by `--ready-timeout` (default `30s`). If the mount path never becomes readable in time, the command fails with `fs.mount_ready_timeout`, the background mount is left running, and the mount locator is preserved so the matching unmount command can stop it. Filesystem layers can fork copy-on-write child timelines without copying a workspace. Layer and checkpoint mounts require FUSE; checkpoint mounts are always read-only. Drive9 does not support combining recursive copy with a layer, so seed a directory tree through a writable layer mount, drain it, and then create the checkpoint: @@ -310,6 +328,7 @@ Structured commands output JSON by default. Use `--output text` for command-spec ```text ti configure ti update +ti exec [flags] [--] command [arg...] ti db create-db-cluster --db-cluster-type starter ti db list-db-clusters --db-cluster-type starter @@ -461,6 +480,7 @@ make live-e2e-fs make live-e2e-fs-git make live-e2e-fs-journal make live-e2e-fs-vault +make live-e2e-runtime ``` Run the complete live suite in one test process: @@ -469,4 +489,4 @@ Run the complete live suite in one test process: make live-e2e ``` -Set `LIVE_E2E_PROFILE=` to use a profile other than `live-e2e`. The DB and FS suites perform real cloud mutations and clean up only resources created by the test run. +Set `LIVE_E2E_PROFILE=` to use a profile other than `live-e2e`. The DB and FS suites perform real cloud mutations and clean up only resources created by the test run. `make live-e2e-runtime` and the complete `make live-e2e` suite require Runtime to be enabled for the selected Filesystem; they run harmless commands, verify separate stdout and stderr plus exact non-zero exit propagation, and fail instead of treating a disabled Runtime as proof of compatibility. diff --git a/cmd/ti/main.go b/cmd/ti/main.go index 5f332b1..a605b7c 100644 --- a/cmd/ti/main.go +++ b/cmd/ti/main.go @@ -24,7 +24,9 @@ func main() { info := version.Current() root := cli.NewRootCommand(info) if err := cli.Execute(ctx, root, info, os.Args[1:], os.Stdout, os.Stderr, cli.WithHomeMigration()); err != nil { - fmt.Fprintf(os.Stderr, "\nti [ERROR]: %s\n", apperr.MessageFor(err)) + if !apperr.IsSilent(err) { + fmt.Fprintf(os.Stderr, "\nti [ERROR]: %s\n", apperr.MessageFor(err)) + } os.Exit(apperr.ExitCodeFor(err)) } } diff --git a/docs/pingcap-docs/docs b/docs/pingcap-docs/docs index 07c3776..aef3619 160000 --- a/docs/pingcap-docs/docs +++ b/docs/pingcap-docs/docs @@ -1 +1 @@ -Subproject commit 07c3776792ff8b135608e49ed7bb4100aee83e38 +Subproject commit aef361990dae579cb6f1c9cf0b007fe7f474f348 diff --git a/docs/priciples.md b/docs/priciples.md index 1eaf760..7995500 100644 --- a/docs/priciples.md +++ b/docs/priciples.md @@ -9,6 +9,7 @@ ti is currently in Preview. Its feature and command contracts can change before - `ti db` manages TiDB Cloud Starter clusters and branches, prepares SQL users, formats connection strings, executes one SQL statement per invocation, lists export tasks, and downloads completed local-target export files. - `ti fs` manages TiDB Cloud Filesystem resources, files, layers, packs, and mounts. - `ti fs-git`, `ti fs-journal`, and `ti fs-vault` expose Filesystem-backed Git workspace, append-only journal, and secret-management workflows. +- `ti exec` runs one command in a remote sandbox with the selected Filesystem mounted at `/workspace`. - `ti configure` initializes a local profile. - `ti update` explicitly checks for or installs a release update. @@ -22,10 +23,10 @@ The command tree has at most two command levels: ti [subcommand] ``` -`configure` and `update` are intentional top-level verb exceptions. Other top-level commands identify product domains: `db`, `fs`, `fs-git`, `fs-journal`, and `fs-vault`. +`configure`, `update`, and `exec` are intentional top-level verb exceptions. Other top-level commands identify product domains: `db`, `fs`, `fs-git`, `fs-journal`, and `fs-vault`. `ti exec` intentionally matches the concise public `drive9 exec` interaction instead of adding a `runtime` parent. - Commands and flags use complete, self-explanatory names. -- Flags are long-only. Do not add one-letter flags. +- Flags are long-only except for the exact `ti exec` compatibility aliases `-w`, `-C`, `-e`, `-t`, and `-v`. - Required flags appear before optional flags in help output. - `ti fs` Unix-style aliases shorten only the command name. Their flags stay long and match the canonical command. - Only `ti configure` may prompt. @@ -39,6 +40,7 @@ ti [subcommand] - `--output json` and `--output text` are the supported structured output modes. - `--query` applies a JMESPath expression after execution and before rendering. - Commands that stream raw bytes reject `--query`. +- `ti exec` streams stdout and stderr separately, rejects `--output` and `--query`, and returns the remote command's exact exit status without adding a structured error line. - Mutating control-plane commands support `--dry-run`; read-only commands reject it. - Dry run validates local input, credentials, placement, and request construction without sending a mutation. - Errors use `ti [ERROR]: ` and stable exit categories. @@ -166,10 +168,11 @@ SQL execution prefers the HTTPS SQL API and uses the selected SQL username/passw ## Filesystem Ownership Boundary -ti does not implement filesystem runtime semantics itself. Installed `ti fs`, `ti fs-git`, `ti fs-journal`, and `ti fs-vault` commands route through the bundled `ti-drive9` companion. +ti does not implement filesystem or remote-execution runtime semantics itself. Installed `ti fs`, `ti fs-git`, `ti fs-journal`, `ti fs-vault`, and `ti exec` commands route through the bundled `ti-drive9` companion. - ti owns command naming, profile and region resolution, resource selection, credential storage, preflight validation, output/query behavior, errors, installation, and updates. - Drive9 owns file data-plane semantics, layers, pack/unpack, FUSE and WebDAV mounts, drain, Git workspace behavior, journal behavior, and Vault behavior. +- Drive9 owns Runtime execution, live streams, local-mount drain and inference, sandbox reuse, cancellation, and unknown-outcome classification. - There is no native ti filesystem fallback. - `ref/drive9` is context only and is never imported, built, packaged, or used by tests. - ti exposes only operations present in the Drive9 public CLI. It does not expose Drive9 internal APIs. @@ -182,11 +185,15 @@ Each resource runs the companion with isolated state under: ti supplies a sanitized companion environment containing the resolved server, canonical region, and resource owner token. Inherited `DRIVE9_*` values must not override ti's selection. Users do not edit `~/.drive9` for ti workflows. +`ti exec [flags] [--] command [arg...]` mirrors Drive9 Runtime flags exactly: `-w/--workspace`, `-C/--cwd`, repeatable `-e/--env`, `-t/--timeout`, `--fresh`, `--layer`, `--read-only`, and `-v/--verbose`. Flag parsing stops at the remote command boundary. Runtime stdout and stderr are never buffered or rendered as structured output, executions are never retried, and exit statuses `124`, `130`, and `255` retain timeout, cancellation, and unknown-outcome meanings. + +`ti exec` first runs a credential-free local `ti-drive9 exec --help` capability gate and accepts only exit status zero; human help text is not an API. Only after that gate succeeds may ti load the selected profile, resolve a File System credential, or start a credential-bearing companion command. Installers and the updater enforce the same command gate before replacing installed binaries. + Filesystem resource selection is: 1. Explicit `--file-system-id` or `TI_FS_FILE_SYSTEM_ID`. 2. The file system ID embedded in an explicitly supplied FS token; any separate ID must match it. -3. Otherwise fail with `fs.missing_file_system_id` before endpoint resolution, companion startup, or a remote call. +3. Otherwise fail with `fs.missing_file_system_id` before endpoint resolution, credential-bearing companion startup, or a remote call. The credential-free local Runtime capability gate is the only `ti exec` exception. ti never infers a target from profile state, the number of local credentials, creation order, or deletion side effects. Creating the first resource does not select it for later commands, and deleting a resource does not promote another resource. `TI_FS_FILE_SYSTEM_ID` is an explicit process-scoped selector, or a consistency assertion when a token is also supplied; it is not a persisted default. diff --git a/docs/releasing.md b/docs/releasing.md index c050156..9eecf1d 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -6,7 +6,7 @@ The tag supplies the binary version through GoReleaser build flags. There is no ## When Drive9 publishes a new CLI -The ti archives contain `ti` and the README. The installers and updater download the `ti-drive9` companion separately, verify its SHA-256 checksum, and check required layer commands and mount flags before installation. +The ti archives contain `ti` and the README. The installers and updater download the `ti-drive9` companion separately, verify its SHA-256 checksum, and check required layer commands, mount flags, and the Runtime exec command before installation. | Consumer | Companion binary source | Checksum file | | --- | --- | --- | @@ -23,7 +23,7 @@ Do not claim that a ti rollback also rolls back Drive9. Windows users must rerun ## Prepare the release PR 1. Check the latest published ti release and changes since that tag. Use the next patch version for a companion refresh with no ti interface change. Include only merged changes in the release notes. -2. Check the Drive9 `version` file and checksum manifest at each source above. Confirm that they agree and contain all five ti targets: Darwin and Linux on amd64/arm64, and Windows on amd64. Download and verify the companion on the validation host, then inspect `ti-drive9 version`, `ti-drive9 fs layer help`, and `ti-drive9 mount --help`. The required capabilities include layer `fork`, `chain`, `delete` and mount `--layer`, `--checkpoint`. +2. Check the Drive9 `version` file and checksum manifest at each source above. Confirm that they agree and contain all five ti targets: Darwin and Linux on amd64/arm64, and Windows on amd64. Download and verify the companion on the validation host, then inspect `ti-drive9 version`, `ti-drive9 fs layer help`, and `ti-drive9 mount --help`, and require `ti-drive9 exec --help` to exit zero. Do not parse Runtime help prose as an API. The required capabilities include layer `fork`, `chain`, `delete`, mount `--layer`, `--checkpoint`, and the Runtime command from Drive9 #1022. Record the exact Drive9 version, full Git hash, and each artifact checksum in the release evidence. Publish the Drive9 artifact before the ti release. 3. Add `docs/release-notes/.md`, following recent notes. For a companion refresh, identify the tested Drive9 build, link the relevant upstream fixes, explain the mutable download source, and include upgrade and reinstall commands. Pass the ti version explicitly to the installer: downloading an installer from a tagged URL alone still defaults to installing the latest ti release. 4. Validate the candidate with the repository commands: @@ -41,7 +41,7 @@ Do not claim that a ti rollback also rolls back Drive9. Windows users must rerun make live-e2e ``` - The default profile is `live-e2e`; use `LIVE_E2E_PROFILE=` to override it. These tests create and clean up temporary cloud resources. Check the output for skips caused by unavailable credentials, quota, or mount prerequisites. Ordinary unit/e2e tests use a fake companion and do not prove live Drive9 compatibility. Focused `make live-e2e-fs`, `make live-e2e-fs-git`, `make live-e2e-fs-journal`, and `make live-e2e-fs-vault` targets help investigate failures but do not replace the full release suite. + The default profile is `live-e2e`; use `LIVE_E2E_PROFILE=` to override it. These tests create and clean up temporary cloud resources. The selected Filesystem must have Runtime enabled. Check the output for skips caused by unavailable credentials, quota, or mount prerequisites. Ordinary unit/e2e tests use a fake companion and do not prove live Drive9 compatibility. A skipped `make live-e2e-runtime` is a failed Runtime release gate, not a pass. Focused `make live-e2e-fs`, `make live-e2e-fs-git`, `make live-e2e-fs-journal`, `make live-e2e-fs-vault`, and `make live-e2e-runtime` targets help investigate failures but do not replace the full release suite. 6. Open a release PR and record the tested companion version, checksums, successful checks, and any validation still required. Merge after review and required checks pass. ## Publish after merge diff --git a/docs/spec/done/0001-cli-foundation.md b/docs/spec/done/0001-cli-foundation.md index 47399e8..0be5400 100644 --- a/docs/spec/done/0001-cli-foundation.md +++ b/docs/spec/done/0001-cli-foundation.md @@ -1,5 +1,10 @@ # CLI Foundation +> **Runtime exception:** `0036-runtime-exec.md` adds top-level `ti exec` and +> its exact `-w`, `-C`, `-e`, `-t`, and `-v` Drive9 compatibility flags. The +> noun-first and long-only rules below remain authoritative for every other +> command. + ## Goal Establish the tdc CLI executable, command tree, global flags, help behavior, diff --git a/docs/spec/done/0036-runtime-exec.md b/docs/spec/done/0036-runtime-exec.md new file mode 100644 index 0000000..3ea8a6d --- /dev/null +++ b/docs/spec/done/0036-runtime-exec.md @@ -0,0 +1,199 @@ +# Runtime Exec + +## Goal + +Expose Drive9 Runtime through one concise top-level command: + +```text +ti exec [flags] [--] command [arg...] +ti exec [flags] 'shell command line' +``` + +The public interaction must match `drive9 exec` from +`mem9-ai/drive9#1022` at exact head +`ce0bb29bf1ad24e0e8e1e0107c138061fdea42b6`. `ti` continues to own +profile, placement, File System selection, credential isolation, companion +discovery, telemetry, and operation-log privacy. The bundled `ti-drive9` +companion owns Runtime request, streaming, local-mount synchronization, +cancellation, sandbox reuse, and outcome classification. + +This is an intentional exception to the usual noun-first top-level command +and long-only flag rules. Adding a `runtime` parent would make the common path +less direct, while changing the established Runtime flag names would create +two incompatible interfaces for the same operation. The exception applies +only to `ti exec`. + +## Command And Flags + +`ti exec` accepts the same Runtime flags as the pinned Drive9 contract: + +| Flag | Meaning | +| --- | --- | +| `-w`, `--workspace PATH` | Drive9 directory to run in; infer an enclosing matching local FUSE mount when omitted, otherwise use the drive root | +| `-C`, `--cwd DIR` | Subdirectory of the selected workspace | +| `-e`, `--env KEY[=VALUE]` | Set a remote variable; a bare key copies the local value; repeatable | +| `-t`, `--timeout DURATION` | Caller deadline; the server may impose a shorter limit | +| `--fresh` | Request a new sandbox instead of warm reuse | +| `--layer REF` | Run against a LayerFS layer ID, name, or tag reference | +| `--read-only` | Mount the workspace read-only | +| `-v`, `--verbose` | Show sandbox, working directory, and duration details | + +It also accepts the ti-owned long flags `--file-system-id` and `--fs-token`, +plus the existing global `--profile`, `--region`, and `--debug` flags. Prefer +`TI_FS_TOKEN` over `--fs-token` so secrets do not enter process arguments. + +Flag parsing stops at the first command argument. Therefore this runs the +remote command with `-v` as its argument rather than enabling ti verbosity: + +```bash +ti exec npm test -- -v +``` + +An explicit `--` remains supported. A single command argument containing +spaces or shell syntax is passed unchanged to Drive9, which runs it through +`/bin/sh -c`; multiple arguments are executed directly. + +`--output`, `--query`, and `--dry-run` are not supported. Runtime output is a +live byte stream, not a structured ti result. `ti exec help` and `ti exec +--help` show command help without loading profiles, credentials, settings +beyond the normal help exclusions, or starting the companion. + +## Resource And Credential Resolution + +Before loading a profile or resolving any credential, run the local +`ti-drive9 exec --help` command with a sanitized environment that contains no +profile, endpoint, placement, or credential. Exit status zero is the command +capability contract; do not parse human help text. An older companion must +fail here before any workspace synchronization or remote request. + +After the local capability gate, resolve exactly one File System using the +existing data-plane contract: + +1. explicit `--file-system-id` or `TI_FS_FILE_SYSTEM_ID`; +2. otherwise derive the ID from explicit `--fs-token` or `TI_FS_TOKEN`; +3. otherwise fail with `fs.missing_file_system_id` before endpoint resolution, + credential-bearing companion startup, or a remote call. + +Resolve the credential from explicit `--fs-token`, then `TI_FS_TOKEN`, then +the selected resource's local credential. Resolve placement from explicit +`--region`, `TI_REGION_CODE`, then the selected resource. Runtime requires an +Drive9 API-key credential. A delegated Vault token cannot run commands; +service authorization remains authoritative for a scoped API key. + +Run the companion with the existing resource-scoped HOME: + +```text +~/.ti/drive9-home// +``` + +Strip inherited `DRIVE9_*`, TiDB Cloud keys, FS tokens, and Vault tokens, then +set only the resolved `DRIVE9_SERVER`, `DRIVE9_REGION_CODE`, and +`DRIVE9_API_KEY`. Do not expose the credential in argv, logs, telemetry, +errors, or command output. + +The companion invocation is equivalent to: + +```text +ti-drive9 exec -- +``` + +The wrapper must never invoke a native ti Runtime client, another transport, +or a retry fallback. An incompatible companion fails before execution with an +actionable reinstall/update error. + +The installers and updater must apply the same successful-command gate before +replacing either installed binary. A release containing this command is not +deliverable until the official Drive9 artifacts contain #1022, their exact +version and checksums are recorded, and the checksum-verified artifact passes +`make live-e2e-runtime` against a Runtime-enabled File System. The required +publication order is Drive9 #1022 artifact, live Runtime validation, then the +ti release. + +## Local Mount Behavior + +Because the companion uses the same resource-scoped HOME and credential as ti +mount commands, Drive9 may infer an enclosing live FUSE mount owned by the +same server and account. It drains pending local writes before starting the +remote command and runs in the corresponding workspace directory. A layer +mount retains its resolved layer. + +Drive9 refuses unverifiable, cross-account, stale, or non-FUSE mounts. In +particular, WebDAV is the default on macOS and Windows, so callers there must +usually pass `-w`. Explicit `-w` selects the remote directory and does not +upload local changes first. + +## Streams, Exit Status, And Cancellation + +- Connect the user's stdin, stdout, and stderr directly to the companion. +- Keep stdout and stderr separate and live; do not buffer, combine, decode, + query, or re-render remote bytes. +- A completed remote command exits with its exact status. Do not add a + `ti [ERROR]` line for a nonzero remote status. +- Timeout exits `124`, first-interrupt cancellation exits `130`, and unknown + outcome exits `255`, using the companion's actionable stderr message. +- Missing/not-executable commands keep shell statuses `127`/`126`. +- A busy or not-enabled rejection is a known pre-start failure. Lost, + malformed, oversized, contradictory, redirected, or interrupted streams + after possible acceptance are `outcome_unknown` and exit `255`. +- Never retry or resubmit an execution. The command may already have run. +- The companion owns the two-stage interrupt protocol: the first Ctrl-C + requests remote cancellation and waits for workspace drain; the second + detaches. The ti parent must not kill the companion on the first interrupt. + +## Privacy And Observability + +Operation logs and telemetry may record only `ti exec`, explicitly used flag +names, region/provider, duration, stable error code, and final exit status. +They must never record command/argument values, environment values, workspace +or cwd values, File System IDs, layer references, execution IDs, stdout, +stderr, API keys, or raw companion errors. Debug output may state that the +Runtime companion is being started, but must omit all Runtime arguments. + +## Package Boundaries + +- `internal/cli` owns command help, ti flag parsing, resource/profile + selection, raw-output incompatibility checks, and the explicit short-flag + exception. +- `internal/runtimeexec` owns deterministic construction of the companion + arguments and one non-retrying execution call. +- `internal/fswrap` owns companion location, isolated HOME/environment + construction, direct stream wiring, and silent process-exit propagation. +- `internal/apperr` owns the silent-exit marker used only when another process + already rendered the result. + +No Runtime HTTP/SSE protocol implementation belongs in ti-cli. + +## Tests + +Unit and compiled black-box tests must prove: + +- exact `ti exec` help and all short/long Runtime flags; +- normal argv, a single shell line, `--`, and flags after the command; +- repeatable environment flags and ti-owned selection flags; +- stdout/stderr remain separate and unmodified; +- remote nonzero, `124`, `130`, and `255` exit statuses are passed through + without an added `ti [ERROR]` line; +- one and only one execution attempt occurs, including stream failure; +- the local capability gate runs before profile/credential resolution, carries + no endpoint or credential, and rejects a companion whose `exec --help` + command exits nonzero without parsing its output; +- missing selection and incompatible companion fail before remote execution; +- resource-scoped HOME and sanitized credential environment are used; +- `--output`, `--query`, and `--dry-run` fail locally; +- short flags remain rejected everywhere except `ti exec`; +- command values, environment values, IDs, paths, and output do not enter + telemetry, operation logs, or debug diagnostics. + +Add an opt-in `live-e2e-runtime` target that executes a harmless command +against a Runtime-enabled real File System, verifies separate stdout/stderr +and a nonzero exit, and never deletes a resource it did not create. A skipped +or disabled Runtime test is not evidence that the live path works. + +## Out Of Scope + +- Runtime provisioning, provider/profile selection, sandbox listing, manual + cancellation by execution ID, logs replay, or recovery commands; +- a `ti runtime` command family; +- a native Runtime SDK or HTTP client in ti-cli; +- retry, local execution, FUSE execution, or upload/download fallback; +- changing Drive9 Runtime server semantics or enabling Runtime in production. diff --git a/e2e/cli_test.go b/e2e/cli_test.go index c61bf26..ec205d1 100644 --- a/e2e/cli_test.go +++ b/e2e/cli_test.go @@ -1232,6 +1232,186 @@ func TestFSConfigurationFreeAccess(t *testing.T) { assertFakeDrive9Call(t, calls, []string{"umount"}, "", home, "default", "tenant-sandbox", "https://fs-east.test", "aws-us-east-1") } +func TestRuntimeExecCommandSurfaceAndFailureContract(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("fake companion process semantics are covered by unit tests on Windows") + } + bin := tiBinary(t) + home := t.TempDir() + companion := filepath.Join(t.TempDir(), "ti-drive9") + build := exec.Command("go", "build", "-o", companion, "./testdata/fake-drive9.go") + build.Dir = "." + if output, err := build.CombinedOutput(); err != nil { + t.Fatalf("build fake Drive9 companion: %v\n%s", err, output) + } + recordPath := filepath.Join(t.TempDir(), "calls.jsonl") + token := drive9TestToken("tenant-runtime") + baseEnv := []string{ + "HOME=" + home, + "TI_LOGGING=on", + "TI_PROFILE=private-runtime-profile", + "TI_DRIVE9_BIN=" + companion, + "FAKE_DRIVE9_RECORD=" + recordPath, + "TI_FS_TOKEN=" + token, + "TI_REGION_CODE=aws-us-east-1", + "DRIVE9_API_KEY=ambient-must-not-win", + } + + helpRecord := filepath.Join(t.TempDir(), "help-calls.jsonl") + helpEnv := []string{ + "HOME=" + t.TempDir(), + "TI_DRIVE9_BIN=" + companion, + "FAKE_DRIVE9_RECORD=" + helpRecord, + } + for _, args := range [][]string{{"exec", "help"}, {"exec", "--help"}} { + help := runTIWithInput(t, bin, "", helpEnv, args...) + help.wantExitCode(0) + help.wantStdoutContains("ti exec [flags] [--] command [arg...]") + for _, flag := range []string{"-w, --workspace", "-C, --cwd", "-e, --env", "-t, --timeout", "--fresh", "--layer", "--read-only", "-v, --verbose", "--file-system-id", "--fs-token"} { + help.wantStdoutContains(flag) + } + for _, unsupported := range []string{"--output", "--query", "--dry-run"} { + help.wantStdoutNotContains(unsupported) + } + } + if _, err := os.Stat(helpRecord); !os.IsNotExist(err) { + t.Fatalf("Runtime help invoked the companion or touched its record file: %v", err) + } + + success := runTIWithInput(t, bin, "", append(baseEnv, "SECRET_FROM_LOCAL=local-value"), + "exec", "-w", "private/workspace", "-C", "web", "-e", "CI=1", "-e", "SECRET_FROM_LOCAL", "-t", "20m", "--fresh", "--layer", "private-layer", "--read-only", "-v", "--", "run-secret-command", "--output", "text") + success.wantExitCode(0) + if success.stdout != "runtime stdout\n" || success.stderr != "runtime stderr\n" { + success.fail("Runtime streams changed or merged") + } + + remoteFlags := runTIWithInput(t, bin, "", baseEnv, "exec", "echo", "-v", "--profile", "remote-profile", "--query", "remote-query", "--debug") + remoteFlags.wantExitCode(0) + if remoteFlags.stderr != "runtime stderr\n" { + remoteFlags.fail("remote --debug unexpectedly enabled ti debug output") + } + shellLine := "printf 'private shell line'" + shell := runTIWithInput(t, bin, "", baseEnv, "exec", shellLine) + shell.wantExitCode(0) + structured := runTIWithInput(t, bin, "", baseEnv, "exec", "--output", "text", "echo") + structured.wantExitCode(2) + structured.wantStderrContains("--output is not supported by `ti exec`") + queried := runTIWithInput(t, bin, "", baseEnv, "exec", "--query", "value", "echo") + queried.wantExitCode(2) + queried.wantStderrContains("--query is not supported by `ti exec`") + dryRun := runTIWithInput(t, bin, "", baseEnv, "exec", "--dry-run", "echo") + dryRun.wantExitCode(2) + dryRun.wantStderrContains("unknown flag: --dry-run") + + for _, test := range []struct { + mode string + code int + }{ + {mode: "nonzero", code: 23}, + {mode: "timeout", code: 124}, + {mode: "canceled", code: 130}, + {mode: "unknown", code: 255}, + } { + result := runTIWithInput(t, bin, "", append(baseEnv, "FAKE_DRIVE9_EXEC_MODE="+test.mode), "exec", "run-once-"+test.mode) + result.wantExitCode(test.code) + result.wantStderrContains("runtime stderr") + result.wantStderrNotContains("ti [ERROR]") + } + + incompatible := runTIWithInput(t, bin, "", append(baseEnv, "FAKE_DRIVE9_EXEC_MODE=incompatible"), "exec", "must-not-run") + incompatible.wantExitCode(1) + incompatible.wantStderrContains("installed ti-drive9 companion does not support `ti exec`") + incompatible.wantStdoutNotContains("runtime stdout") + + incompatibleWithoutSelectionRecord := filepath.Join(t.TempDir(), "incompatible-without-selection.jsonl") + incompatibleWithoutSelection := runTIWithInput(t, bin, "", []string{ + "HOME=" + t.TempDir(), "TI_DRIVE9_BIN=" + companion, "FAKE_DRIVE9_RECORD=" + incompatibleWithoutSelectionRecord, + "FAKE_DRIVE9_EXEC_MODE=incompatible", "TI_REGION_CODE=aws-us-east-1", + }, "exec", "must-not-resolve") + incompatibleWithoutSelection.wantExitCode(1) + incompatibleWithoutSelection.wantStderrContains("installed ti-drive9 companion does not support `ti exec`") + incompatibleWithoutSelection.wantStderrNotContains("file system ID is required") + incompatibleWithoutSelectionCalls := readFakeDrive9Calls(t, incompatibleWithoutSelectionRecord) + if len(incompatibleWithoutSelectionCalls) != 1 || strings.Join(incompatibleWithoutSelectionCalls[0].Args, "\x00") != "exec\x00--help" { + t.Fatalf("incompatible companion passed preflight before resource resolution: %#v", incompatibleWithoutSelectionCalls) + } + + missingRecord := filepath.Join(t.TempDir(), "missing.jsonl") + missing := runTIWithInput(t, bin, "", []string{ + "HOME=" + t.TempDir(), "TI_DRIVE9_BIN=" + companion, "FAKE_DRIVE9_RECORD=" + missingRecord, "TI_REGION_CODE=aws-us-east-1", + }, "exec", "must-not-start") + missing.wantExitCode(2) + missing.wantStderrContains("file system ID is required") + missingCalls := readFakeDrive9Calls(t, missingRecord) + if len(missingCalls) != 1 || strings.Join(missingCalls[0].Args, "\x00") != "exec\x00--help" { + t.Fatalf("missing selection passed the local capability gate: %#v", missingCalls) + } + if missingCalls[0].APIKey != "" || missingCalls[0].Server != "" || missingCalls[0].RegionCode != "" { + t.Fatalf("capability gate received credentials or placement before selection: %#v", missingCalls[0]) + } + + calls := readFakeDrive9Calls(t, recordPath) + assertFakeDrive9Args(t, calls, []string{"exec", "--workspace", "private/workspace", "--cwd", "web", "--env", "CI=1", "--env", "SECRET_FROM_LOCAL", "--timeout", "20m", "--fresh", "--layer", "private-layer", "--read-only", "--verbose", "--", "run-secret-command", "--output", "text"}) + assertFakeDrive9Args(t, calls, []string{"exec", "--", "echo", "-v", "--profile", "remote-profile", "--query", "remote-query", "--debug"}) + assertFakeDrive9Args(t, calls, []string{"exec", "--", shellLine}) + for _, call := range calls { + if len(call.Args) < 1 || call.Args[0] != "exec" { + continue + } + if len(call.Args) > 1 && call.Args[1] == "--help" { + if call.APIKey != "" || call.Server != "" || call.RegionCode != "" || call.TIFSToken != "" || call.TiDBCloudPublicKey != "" || call.TiDBCloudPrivateKey != "" || call.Drive9Public != "" || call.Drive9Private != "" { + t.Fatalf("Runtime capability gate received credentials or placement: %#v", call) + } + continue + } + if call.APIKey != token || call.TIFSToken != "" || call.TiDBCloudPublicKey != "" || call.TiDBCloudPrivateKey != "" || call.Drive9Public != "" || call.Drive9Private != "" { + t.Fatalf("Runtime companion credential boundary failed: %#v", call) + } + } + + logData, err := os.ReadFile(filepath.Join(home, ".ti", "logs", "ti.jsonl")) + if err != nil { + t.Fatalf("read Runtime operation log: %v", err) + } + for _, forbidden := range []string{token, "private-runtime-profile", "private/workspace", "private-layer", "run-secret-command", shellLine, "SECRET_FROM_LOCAL", "local-value", "runtime stdout", "runtime stderr"} { + if strings.Contains(string(logData), forbidden) { + t.Fatalf("Runtime operation log leaked %q", forbidden) + } + } + + telemetryRequests := make(chan []byte, 1) + telemetryServer := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + body, _ := io.ReadAll(request.Body) + telemetryRequests <- body + writer.WriteHeader(http.StatusAccepted) + })) + defer telemetryServer.Close() + telemetryCommand := "runtime-secret-command" + telemetryWorkspace := "private/telemetry-workspace" + telemetryEnv := "RUNTIME_SECRET=secret-value" + telemetry := runTIWithInput(t, bin, "", append(baseEnv, + "TI_ALLOW_TEST_ENDPOINTS=1", + "TI_TEST_TELEMETRY_ENDPOINT="+telemetryServer.URL, + "TI_TELEMETRY=on", + ), "exec", "-w", telemetryWorkspace, "-e", telemetryEnv, telemetryCommand, "--debug") + telemetry.wantExitCode(0) + var telemetryBody []byte + select { + case telemetryBody = <-telemetryRequests: + case <-time.After(2 * time.Second): + t.Fatal("ti exec did not send telemetry to the fake ingestion server") + } + for _, forbidden := range []string{token, telemetryCommand, telemetryWorkspace, telemetryEnv, "secret-value", "--debug", "runtime stdout", "runtime stderr"} { + if strings.Contains(string(telemetryBody), forbidden) { + t.Fatalf("Runtime telemetry leaked %q: %s", forbidden, telemetryBody) + } + } + if !strings.Contains(string(telemetryBody), `"command_path":"ti exec"`) || + !strings.Contains(string(telemetryBody), `"flag_names":["env","workspace"]`) { + t.Fatalf("unexpected Runtime telemetry payload: %s", telemetryBody) + } +} + func TestFSLayerForkWorkflowCommands(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("fake companion build path is covered by unit tests on Windows") @@ -2128,7 +2308,7 @@ func runTI(t *testing.T, bin string, args ...string) commandResult { func hasLiveFSCommandFamily(args []string) bool { for _, arg := range args { switch arg { - case "fs", "fs-git", "fs-journal", "fs-vault": + case "exec", "fs", "fs-git", "fs-journal", "fs-vault": return true } } diff --git a/e2e/installer_test.go b/e2e/installer_test.go index 0507bc0..64d126a 100644 --- a/e2e/installer_test.go +++ b/e2e/installer_test.go @@ -203,7 +203,7 @@ fi if err != nil { t.Fatalf("read companion validation calls: %v", err) } - for _, want := range []string{"fs layer help", "mount --help"} { + for _, want := range []string{"fs layer help", "mount --help", "exec --help"} { if !strings.Contains(string(companionCallData), want) { t.Fatalf("installer did not validate companion command surface %q; calls:\n%s", want, companionCallData) } @@ -245,6 +245,9 @@ if [ "$1 $2" = "mount --help" ]; then printf ' -layer string\n -checkpoint string\n' exit 0 fi +if [ "$1 $2" = "exec --help" ]; then + exit 0 +fi printf 'compatible companion\n' `, callLog) } @@ -324,7 +327,7 @@ func TestInstallersValidateCompanionCommandSurfaceBeforeInstallation(t *testing. if powerShellValidation < 0 || powerShellInstall < 0 || powerShellValidation >= powerShellInstall { t.Fatal("install.ps1 must validate the staged companion before installing it") } - for _, required := range []string{"fork", "chain", "delete", "layer", "checkpoint"} { + for _, required := range []string{"fork", "chain", "delete", "layer", "checkpoint", "exec"} { if !strings.Contains(shell, required) || !strings.Contains(powerShell, required) { t.Fatalf("installers do not validate required companion surface %q", required) } diff --git a/e2e/live_test.go b/e2e/live_test.go index ea52519..b5a168d 100644 --- a/e2e/live_test.go +++ b/e2e/live_test.go @@ -193,6 +193,28 @@ func TestLiveCLICommandSurface(t *testing.T) { checkUpdateHelp.wantStdoutNotContains("--yes") } +func TestLiveRuntimeExec(t *testing.T) { + requireLive(t) + if os.Getenv("TI_LIVE_RUNTIME") != "1" { + t.Skip("TI_LIVE_RUNTIME=1 is required; run make live-e2e-runtime against a Runtime-enabled File System") + } + + bin := tiBinary(t) + profileName := liveProfileName(t) + profile := ensureLiveFSResource(t, bin, profileName) + env := liveFSTokenEnv(profile, t.TempDir()) + + success := runTIWithInput(t, bin, "", env, "exec", "printf 'ti-runtime-stdout\\n'; printf 'ti-runtime-stderr\\n' >&2") + success.wantExitCode(0) + if success.stdout != "ti-runtime-stdout\n" || success.stderr != "ti-runtime-stderr\n" { + success.fail("live Runtime stdout and stderr changed or merged") + } + + nonzero := runTIWithInput(t, bin, "", env, "exec", "exit 17") + nonzero.wantExitCode(17) + nonzero.wantStderrNotContains("ti [ERROR]") +} + func TestLiveDBCommandSurface(t *testing.T) { requireLive(t) bin := tiBinary(t) diff --git a/e2e/runtime_signal_unix_test.go b/e2e/runtime_signal_unix_test.go new file mode 100644 index 0000000..b71d352 --- /dev/null +++ b/e2e/runtime_signal_unix_test.go @@ -0,0 +1,79 @@ +//go:build !windows + +package e2e + +import ( + "bufio" + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +func TestRuntimeExecFirstInterruptReachesCompanion(t *testing.T) { + bin := tiBinary(t) + home := t.TempDir() + companion := filepath.Join(t.TempDir(), "ti-drive9") + build := exec.Command("go", "build", "-o", companion, "./testdata/fake-drive9.go") + build.Dir = "." + if output, err := build.CombinedOutput(); err != nil { + t.Fatalf("build fake Drive9 companion: %v\n%s", err, output) + } + + cmd := exec.Command(bin, "exec", "wait") + cmd.Env = append(os.Environ(), + "HOME="+home, + "TI_LOGGING=off", + "TI_DRIVE9_BIN="+companion, + "TI_FS_TOKEN="+drive9TestToken("tenant-runtime-signal"), + "TI_REGION_CODE=aws-us-east-1", + "FAKE_DRIVE9_EXEC_MODE=wait-for-signal", + ) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + + ready := make(chan struct{}) + done := make(chan struct{}) + var stdout bytes.Buffer + go func() { + defer close(done) + scanner := bufio.NewScanner(stdoutPipe) + for scanner.Scan() { + stdout.WriteString(scanner.Text() + "\n") + if scanner.Text() == "runtime ready" { + close(ready) + } + } + }() + select { + case <-ready: + case <-time.After(10 * time.Second): + _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + _ = cmd.Wait() + t.Fatalf("Runtime companion did not become ready; stdout=%q stderr=%q", stdout.String(), stderr.String()) + } + if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGINT); err != nil { + t.Fatal(err) + } + err = cmd.Wait() + <-done + exitErr, ok := err.(*exec.ExitError) + if !ok || exitErr.ExitCode() != 130 { + t.Fatalf("ti exec after first interrupt = %v; stdout=%q stderr=%q", err, stdout.String(), stderr.String()) + } + if !strings.Contains(stderr.String(), "runtime canceled") || strings.Contains(stderr.String(), "ti [ERROR]") { + t.Fatalf("first interrupt did not preserve companion cancellation: %q", stderr.String()) + } +} diff --git a/e2e/testdata/fake-drive9.go b/e2e/testdata/fake-drive9.go index 7fd0e39..ab34854 100644 --- a/e2e/testdata/fake-drive9.go +++ b/e2e/testdata/fake-drive9.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "os/signal" "sort" "strings" ) @@ -51,6 +52,37 @@ func main() { _ = file.Close() } args := os.Args[1:] + if hasPrefix(args, "exec", "--help") { + if os.Getenv("FAKE_DRIVE9_EXEC_MODE") == "incompatible" { + fmt.Fprintln(os.Stderr, "drive9: unknown command exec") + os.Exit(2) + } + fmt.Fprintln(os.Stderr, "usage: drive9 exec [flags] [--] command [arg...]") + return + } + if len(args) > 0 && args[0] == "exec" { + fmt.Fprintln(os.Stdout, "runtime stdout") + fmt.Fprintln(os.Stderr, "runtime stderr") + switch os.Getenv("FAKE_DRIVE9_EXEC_MODE") { + case "wait-for-signal": + signals := make(chan os.Signal, 1) + signal.Notify(signals, os.Interrupt) + defer signal.Stop(signals) + fmt.Fprintln(os.Stdout, "runtime ready") + <-signals + fmt.Fprintln(os.Stderr, "runtime canceled") + os.Exit(130) + case "nonzero": + os.Exit(23) + case "timeout": + os.Exit(124) + case "canceled": + os.Exit(130) + case "unknown": + os.Exit(255) + } + return + } if hasPrefix(args, "fs", "layer", "help") { fmt.Println("usage: drive9 fs layer ") return diff --git a/internal/apperr/apperr.go b/internal/apperr/apperr.go index 2c43685..c4dae00 100644 --- a/internal/apperr/apperr.go +++ b/internal/apperr/apperr.go @@ -12,6 +12,7 @@ type Error struct { ExitCode int Message string Cause error + Silent bool } type appErrorer interface { @@ -54,6 +55,26 @@ func Wrap(code, category string, exitCode int, message string, cause error) *Err } } +// SilentExit returns an error that carries a process exit status without +// asking the ti boundary to print another diagnostic. It is used when a child +// process has already written the complete user-facing result to stderr. +func SilentExit(code string, exitCode int) *Error { + return &Error{ + Code: code, + Category: "runtime", + ExitCode: exitCode, + Silent: true, + } +} + +func IsSilent(err error) bool { + if err == nil { + return false + } + var appErr *Error + return errors.As(err, &appErr) && appErr.Silent +} + func NotImplemented(commandPath string) *Error { return New( "cli.not_implemented", diff --git a/internal/cli/exec.go b/internal/cli/exec.go new file mode 100644 index 0000000..64bfef5 --- /dev/null +++ b/internal/cli/exec.go @@ -0,0 +1,153 @@ +package cli + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/tidbcloud/ti-cli/internal/apperr" + "github.com/tidbcloud/ti-cli/internal/runtimeexec" + "github.com/tidbcloud/ti-cli/internal/version" +) + +type execFlags struct { + workspace string + cwd string + environment []string + timeout string + fresh bool + layer string + readOnly bool + verbose bool +} + +func newExecCommand(info version.Info) *cobra.Command { + flags := &execFlags{} + cmd := newCommand(commandSpec{ + Use: "exec [flags] [--] command [arg...]", + Short: "Run a command in a sandbox with the selected Filesystem at /workspace.", + Long: `Run one command in a remote sandbox with the selected TiDB Cloud Filesystem at /workspace. + +Output streams live and the remote command's exit status is preserved. Inside +a matching local FUSE mount, ti uploads pending changes and runs in the same +directory. Elsewhere, use -w to select a remote workspace directory. Runtime +executions are never retried because a lost response may mean the command ran.`, + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return apperr.New("runtime.missing_command", "usage", 2, "missing command; see `ti exec --help`") + } + for _, name := range []string{"output", "query"} { + if flag := cmd.Flag(name); flag != nil && flag.Changed { + return apperr.New("runtime.structured_output_not_supported", "usage", 2, fmt.Sprintf("--%s is not supported by `ti exec`; Runtime stdout and stderr stream unchanged", name)) + } + } + + ctx := commandContext{cmd: cmd} + debug, err := ctx.BoolFlag("debug") + if err != nil { + return err + } + service := runtimeexec.Service{ + Stdin: cmd.InOrStdin(), Stdout: cmd.OutOrStdout(), Stderr: cmd.ErrOrStderr(), + Debug: debug, DebugWriter: cmd.ErrOrStderr(), + } + ready, err := service.Preflight(cmd.Context()) + if err != nil { + return err + } + profile, err := ctx.LoadLocalProfile() + if err != nil { + return err + } + profile, err = fsResolveAuthenticatedProfile(ctx, profile, true) + if err != nil { + return err + } + return ready.Exec(cmd.Context(), runtimeexec.Options{ + Profile: profile, + Workspace: flags.workspace, + WorkspaceSet: cmd.Flags().Changed("workspace"), + Cwd: flags.cwd, + CwdSet: cmd.Flags().Changed("cwd"), + Environment: append([]string(nil), flags.environment...), + Timeout: flags.timeout, + TimeoutSet: cmd.Flags().Changed("timeout"), + Fresh: flags.fresh, + Layer: flags.layer, + LayerSet: cmd.Flags().Changed("layer"), + ReadOnly: flags.readOnly, + Verbose: flags.verbose, + Command: append([]string(nil), args...), + }) + }, + }, info) + cmd.Args = func(_ *cobra.Command, _ []string) error { return nil } + cmd.Flags().StringVarP(&flags.workspace, "workspace", "w", "", "Drive9 directory to run in; default: the matching local FUSE mount directory, otherwise the drive root.") + cmd.Flags().StringVarP(&flags.cwd, "cwd", "C", "", "Subdirectory of the selected workspace to run in.") + cmd.Flags().StringArrayVarP(&flags.environment, "env", "e", nil, "Set KEY=VALUE remotely, or copy a local KEY; repeatable.") + cmd.Flags().StringVarP(&flags.timeout, "timeout", "t", "", "Stop the command after this duration; the server may use a shorter limit.") + cmd.Flags().BoolVar(&flags.fresh, "fresh", false, "Start in a new sandbox instead of reusing a warm sandbox.") + cmd.Flags().StringVar(&flags.layer, "layer", "", "Use a LayerFS layer ID, name, or tag:key=value reference.") + cmd.Flags().BoolVar(&flags.readOnly, "read-only", false, "Mount the workspace read-only.") + cmd.Flags().BoolVarP(&flags.verbose, "verbose", "v", false, "Show the sandbox, working directory, and duration.") + cmd.Flags().String("file-system-id", "", "The file system ID. Can also be supplied through TI_FS_FILE_SYSTEM_ID or derived from an explicitly supplied FS token.") + cmd.Flags().String("fs-token", "", "File system token. Prefer TI_FS_TOKEN so the secret does not enter process arguments.") + cmd.Flags().SetInterspersed(false) + return cmd +} + +func isExecInvocation(args []string) bool { + _, command := invocationCommandIndex(args) + return command == "exec" +} + +func invocationCommandIndex(args []string) (int, string) { + valueFlags := map[string]struct{}{ + "profile": {}, "region": {}, "output": {}, "query": {}, + } + for index := 0; index < len(args); index++ { + arg := args[index] + if arg == "--" { + return -1, "" + } + if strings.HasPrefix(arg, "--") { + name, _, hasValue := strings.Cut(strings.TrimPrefix(arg, "--"), "=") + if _, ok := valueFlags[name]; ok && !hasValue && index+1 < len(args) { + index++ + } + continue + } + if strings.HasPrefix(arg, "-") { + return -1, "" + } + return index, arg + } + return -1, "" +} + +func execMetadataArgs(args []string) []string { + execIndex, command := invocationCommandIndex(args) + if command != "exec" { + return args + } + valueFlags := map[string]struct{}{ + "w": {}, "workspace": {}, "C": {}, "cwd": {}, "e": {}, "env": {}, + "t": {}, "timeout": {}, "layer": {}, "file-system-id": {}, "fs-token": {}, + "profile": {}, "region": {}, "output": {}, "query": {}, + } + for index := execIndex + 1; index < len(args); index++ { + arg := args[index] + if arg == "--" { + return args[:index] + } + if !strings.HasPrefix(arg, "-") || arg == "-" { + return args[:index] + } + name := strings.TrimLeft(arg, "-") + name, _, hasValue := strings.Cut(name, "=") + if _, ok := valueFlags[name]; ok && !hasValue && index+1 < len(args) { + index++ + } + } + return args +} diff --git a/internal/cli/exec_test.go b/internal/cli/exec_test.go new file mode 100644 index 0000000..2ee7121 --- /dev/null +++ b/internal/cli/exec_test.go @@ -0,0 +1,60 @@ +package cli + +import ( + "slices" + "testing" +) + +func TestInvocationCommandIndexSkipsGlobalFlagValues(t *testing.T) { + tests := []struct { + name string + args []string + index int + command string + }{ + {name: "plain", args: []string{"exec", "echo"}, index: 0, command: "exec"}, + {name: "global value", args: []string{"--profile", "stage", "--debug", "exec", "echo"}, index: 3, command: "exec"}, + {name: "global attached value", args: []string{"--profile=stage", "exec", "echo"}, index: 1, command: "exec"}, + {name: "flag value named exec", args: []string{"--profile", "exec", "fs", "help"}, index: 2, command: "fs"}, + {name: "invalid short global", args: []string{"-p", "stage", "exec", "echo"}, index: -1}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + index, command := invocationCommandIndex(test.args) + if index != test.index || command != test.command { + t.Fatalf("invocationCommandIndex(%#v) = (%d, %q), want (%d, %q)", test.args, index, command, test.index, test.command) + } + }) + } +} + +func TestExecMetadataArgsStopBeforeRemoteCommand(t *testing.T) { + tests := []struct { + name string + args []string + want []string + }{ + { + name: "implicit command boundary", + args: []string{"--profile", "stage", "exec", "-e", "CI=1", "echo", "--debug", "--help"}, + want: []string{"--profile", "stage", "exec", "-e", "CI=1"}, + }, + { + name: "explicit command boundary", + args: []string{"exec", "--fresh", "--", "--debug", "--version"}, + want: []string{"exec", "--fresh"}, + }, + { + name: "non exec unchanged", + args: []string{"fs", "list-files", "--debug"}, + want: []string{"fs", "list-files", "--debug"}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if got := execMetadataArgs(test.args); !slices.Equal(got, test.want) { + t.Fatalf("execMetadataArgs(%#v) = %#v, want %#v", test.args, got, test.want) + } + }) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index ed788b1..db0ff4b 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -34,6 +34,7 @@ const usageRequiredFlagAnnotation = "ti_usage_required" func init() { cobra.AddTemplateFunc("usageSynopsis", usageSynopsis) cobra.AddTemplateFunc("formattedFlagUsages", formattedFlagUsages) + cobra.AddTemplateFunc("formattedFlagUsagesFor", formattedFlagUsagesFor) } type Options struct { @@ -88,6 +89,7 @@ func NewRootCommand(info version.Info) *cobra.Command { root.AddCommand(newConfigureCommand(info)) root.AddCommand(newUpdateCommand(info)) + root.AddCommand(newExecCommand(info)) root.AddCommand(newDBCommand(info)) root.AddCommand(newFSCommand(info)) root.AddCommand(newFSGitCommand(info)) @@ -201,6 +203,7 @@ func hasBusinessSubcommands(cmd *cobra.Command) bool { } func hasTelemetryExclusionFlag(args []string) bool { + args = execMetadataArgs(args) for _, arg := range args { if arg == "--" { return false @@ -335,6 +338,7 @@ func isUpdateInvocation(args []string) bool { } func debugRequested(args []string) bool { + args = execMetadataArgs(args) for _, arg := range args { if arg == "--debug" || strings.EqualFold(arg, "--debug=true") { return true @@ -348,6 +352,9 @@ func commandEvent(root, cmd *cobra.Command, err error, duration time.Duration) o cmd = root } profileName, regionCode := commandProfileSummary(cmd) + if cmd != nil && cmd.Name() == "exec" { + profileName = "" + } return oplog.Event{ Type: "command", Version: rootAnnotation(root, "ti.version"), @@ -423,6 +430,9 @@ func commandProfileSummary(cmd *cobra.Command) (string, string) { } func rejectShortFlags(args []string) error { + if isExecInvocation(args) { + return nil + } for _, arg := range args { if arg == "--" { return nil @@ -818,9 +828,12 @@ func normalizeError(err error) error { } } -func HasShorthand(cmd *cobra.Command) bool { +func HasNonExecShorthand(cmd *cobra.Command) bool { found := false visitCommands(cmd, func(current *cobra.Command) { + if current.Name() == "exec" { + return + } for _, set := range []*pflag.FlagSet{ current.Flags(), current.PersistentFlags(), @@ -890,13 +903,16 @@ func usageSynopsis(cmd *cobra.Command) string { return "" } lines := []string{" " + usageCommandLine(cmd)} - lines = append(lines, usageFlagLines(cmd.LocalFlags())...) - lines = append(lines, usageFlagLines(cmd.InheritedFlags())...) + lines = append(lines, usageFlagLinesFor(cmd, cmd.LocalFlags())...) + lines = append(lines, usageFlagLinesFor(cmd, cmd.InheritedFlags())...) return strings.Join(lines, "\n") } func usageCommandLine(cmd *cobra.Command) string { line := cmd.CommandPath() + if cmd.Name() == "exec" { + return line + " [flags] [--] command [arg...]" + } if cmd.HasAvailableSubCommands() { return line + " [command]" } @@ -904,13 +920,17 @@ func usageCommandLine(cmd *cobra.Command) string { } func usageFlagLines(flags *pflag.FlagSet) []string { + return usageFlagLinesFor(nil, flags) +} + +func usageFlagLinesFor(cmd *cobra.Command, flags *pflag.FlagSet) []string { if flags == nil { return nil } required := make([]string, 0) optional := make([]string, 0) flags.VisitAll(func(flag *pflag.Flag) { - if flag == nil || flag.Hidden { + if !usageFlagVisible(cmd, flag) { return } line := " " + usageFlagSegment(flag) @@ -933,6 +953,9 @@ func usageFlagRequired(flag *pflag.Flag) bool { func usageFlagSegment(flag *pflag.Flag) string { segment := "--" + flag.Name + if flag.Shorthand != "" { + segment = "-" + flag.Shorthand + ", " + segment + } if flag.NoOptDefVal != "" { return segment } @@ -961,6 +984,10 @@ func usageFlagValueType(flag *pflag.Flag) string { } func formattedFlagUsages(flags *pflag.FlagSet) string { + return formattedFlagUsagesFor(nil, flags) +} + +func formattedFlagUsagesFor(cmd *cobra.Command, flags *pflag.FlagSet) string { if flags == nil { return "" } @@ -971,7 +998,7 @@ func formattedFlagUsages(flags *pflag.FlagSet) string { lines := make([]flagUsage, 0) maxLabelLength := 0 flags.VisitAll(func(flag *pflag.Flag) { - if flag == nil || flag.Hidden { + if !usageFlagVisible(cmd, flag) { return } label := " " + usageFlagSegment(flag) @@ -996,6 +1023,13 @@ func formattedFlagUsages(flags *pflag.FlagSet) string { return result.String() } +func usageFlagVisible(cmd *cobra.Command, flag *pflag.Flag) bool { + if flag == nil || flag.Hidden { + return false + } + return cmd == nil || cmd.Name() != "exec" || (flag.Name != "output" && flag.Name != "query") +} + func formattedFlagDefault(flag *pflag.Flag) string { if flag == nil || flagDefaultIsZero(flag) { return "" @@ -1042,10 +1076,10 @@ Commands: {{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}} Flags: -{{formattedFlagUsages .LocalFlags | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}} +{{formattedFlagUsagesFor . .LocalFlags | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}} Global Flags: -{{formattedFlagUsages .InheritedFlags | trimTrailingWhitespaces}}{{end}} +{{formattedFlagUsagesFor . .InheritedFlags | trimTrailingWhitespaces}}{{end}} ` const usageTemplate = `Usage: @@ -1053,8 +1087,8 @@ const usageTemplate = `Usage: {{.CommandPath}} help [command]{{end}}{{if .HasAvailableLocalFlags}} Flags: -{{formattedFlagUsages .LocalFlags | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}} +{{formattedFlagUsagesFor . .LocalFlags | trimTrailingWhitespaces}}{{end}}{{if .HasAvailableInheritedFlags}} Global Flags: -{{formattedFlagUsages .InheritedFlags | trimTrailingWhitespaces}}{{end}} +{{formattedFlagUsagesFor . .InheritedFlags | trimTrailingWhitespaces}}{{end}} ` diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 5db9b7d..cb01041 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -745,8 +745,8 @@ func TestNoCommandDefinesShortFlags(t *testing.T) { cmd.InitDefaultHelpFlag() cmd.InitDefaultVersionFlag() }) - if HasShorthand(root) { - t.Fatal("expected command tree to avoid short flags") + if HasNonExecShorthand(root) { + t.Fatal("expected every command except ti exec to avoid short flags") } } diff --git a/internal/fs/drive9_companion_test.go b/internal/fs/drive9_companion_test.go index bc99be3..de43d12 100644 --- a/internal/fs/drive9_companion_test.go +++ b/internal/fs/drive9_companion_test.go @@ -568,7 +568,7 @@ func TestDrive9MissingCompanionIsActionable(t *testing.T) { if err == nil { t.Fatal("expected missing companion error") } - if message := apperr.MessageFor(err); !strings.Contains(message, "ti fs requires the Drive9 companion binary") { + if message := apperr.MessageFor(err); !strings.Contains(message, "ti requires the Drive9 companion binary") { t.Fatalf("unexpected error: %q", message) } } diff --git a/internal/fswrap/runner.go b/internal/fswrap/runner.go index 27b6824..c34e68f 100644 --- a/internal/fswrap/runner.go +++ b/internal/fswrap/runner.go @@ -108,6 +108,56 @@ func (r Runner) Run(ctx context.Context, opts RunOptions) (Result, error) { return Result{CompanionPath: path, Stdout: stdout.Bytes(), Stderr: stderr.Bytes()}, nil } +// RunStreaming starts one companion command with direct stdin/stdout/stderr +// wiring. It deliberately does not bind the process lifetime to ctx: the +// Runtime companion owns the first-interrupt cancel-and-drain protocol and a +// second interrupt detaches. The parent and child receive terminal signals in +// the same foreground process group, so killing the child on ctx cancellation +// would turn the first Ctrl-C into an unsafe detach. +func (r Runner) RunStreaming(ctx context.Context, opts RunOptions) error { + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + path, err := r.locateCompanion() + if err != nil { + return err + } + homeDir, err := r.homeDir(opts) + if err != nil { + return err + } + if err := prepareHome(homeDir); err != nil { + return err + } + env, err := r.drive9Env(homeDir, opts) + if err != nil { + return err + } + + cmd := exec.Command(path, opts.Args...) + cmd.Env = env + cmd.Stdin = r.Stdin + cmd.Stdout = r.Stdout + cmd.Stderr = r.Stderr + if r.Debug && r.DebugWriter != nil { + _, _ = fmt.Fprintln(r.DebugWriter, "ti [DEBUG]: starting Runtime companion") + } + if err := cmd.Start(); err != nil { + return apperr.Wrap("runtime.companion_failed", "runtime", 1, "cannot start the Runtime companion; reinstall ti or set TI_DRIVE9_BIN to a compatible drive9 binary", err) + } + if err := cmd.Wait(); err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() > 0 { + return apperr.SilentExit("runtime.exec_exit", exitErr.ExitCode()) + } + return apperr.New("runtime.outcome_unknown", "runtime", 255, "the Runtime companion ended without a confirmed result; the command may have run, so inspect the workspace before retrying") + } + return nil +} + func (r Runner) CompanionInfo(ctx context.Context, profile *config.Profile) (CompanionInfo, error) { result, err := r.Run(ctx, RunOptions{ Profile: profile, @@ -207,7 +257,7 @@ func (r Runner) locateCompanion() (string, error) { return resolved, nil } } - return "", apperr.New("fs.companion_missing", "runtime", 1, "ti fs requires the Drive9 companion binary; reinstall ti or set TI_DRIVE9_BIN to a compatible drive9 binary") + return "", apperr.New("fs.companion_missing", "runtime", 1, "ti requires the Drive9 companion binary; reinstall ti or set TI_DRIVE9_BIN to a compatible drive9 binary") } func (r Runner) homeDir(opts RunOptions) (string, error) { diff --git a/internal/runtimeexec/service.go b/internal/runtimeexec/service.go new file mode 100644 index 0000000..76b5185 --- /dev/null +++ b/internal/runtimeexec/service.go @@ -0,0 +1,155 @@ +package runtimeexec + +import ( + "context" + "errors" + "io" + + "github.com/tidbcloud/ti-cli/internal/api/endpoints" + "github.com/tidbcloud/ti-cli/internal/apperr" + "github.com/tidbcloud/ti-cli/internal/config" + "github.com/tidbcloud/ti-cli/internal/fswrap" +) + +type Service struct { + Resolver endpoints.Resolver + HomeDir string + CompanionPath string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer + Debug bool + DebugWriter io.Writer +} + +type Options struct { + Profile *config.Profile + Workspace string + WorkspaceSet bool + Cwd string + CwdSet bool + Environment []string + Timeout string + TimeoutSet bool + Fresh bool + Layer string + LayerSet bool + ReadOnly bool + Verbose bool + Command []string +} + +// ReadyService is returned only after the local companion has proved that it +// recognizes the Runtime command. Keeping preflight separate lets callers run +// it before loading a profile or resolving any filesystem credential. +type ReadyService struct { + service Service +} + +func (s Service) Exec(ctx context.Context, opts Options) error { + if len(opts.Command) == 0 { + return apperr.New("runtime.missing_command", "usage", 2, "missing command; see `ti exec --help`") + } + ready, err := s.Preflight(ctx) + if err != nil { + return err + } + return ready.Exec(ctx, opts) +} + +// Preflight proves command availability using only the local companion. It +// intentionally supplies no profile, endpoint, or credential and accepts the +// command's exit status as the contract instead of parsing human help text. +func (s Service) Preflight(ctx context.Context) (ReadyService, error) { + runner := fswrap.Runner{ + HomeDir: s.HomeDir, + CompanionPath: s.CompanionPath, + Resolver: s.Resolver, + Stdin: s.Stdin, + Stdout: s.Stdout, + Stderr: s.Stderr, + Debug: s.Debug, + DebugWriter: s.DebugWriter, + } + if err := requireExecCapability(ctx, runner); err != nil { + return ReadyService{}, err + } + return ReadyService{service: s}, nil +} + +// Exec runs exactly one Runtime command after a successful local preflight. +func (r ReadyService) Exec(ctx context.Context, opts Options) error { + if opts.Profile == nil { + return apperr.New("fs.missing_profile", "config", 2, "active profile is required") + } + if len(opts.Command) == 0 { + return apperr.New("runtime.missing_command", "usage", 2, "missing command; see `ti exec --help`") + } + homeDir := r.service.HomeDir + if homeDir == "" { + homeDir = opts.Profile.HomeDir + } + runner := fswrap.Runner{ + HomeDir: homeDir, + CompanionPath: r.service.CompanionPath, + Resolver: r.service.Resolver, + Stdin: r.service.Stdin, + Stdout: r.service.Stdout, + Stderr: r.service.Stderr, + Debug: r.service.Debug, + DebugWriter: r.service.DebugWriter, + } + return runner.RunStreaming(ctx, fswrap.RunOptions{ + Profile: opts.Profile, + ResourceName: opts.Profile.FSResourceName, + Args: companionArgs(opts), + IncludeFSAPIKey: true, + }) +} + +func requireExecCapability(ctx context.Context, runner fswrap.Runner) error { + runner.Debug = false + runner.DebugWriter = nil + _, err := runner.Run(ctx, fswrap.RunOptions{ + ResourceName: "runtime-capability", + Args: []string{"exec", "--help"}, + CaptureStdout: true, + }) + if err == nil { + return nil + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return err + } + if err != nil && apperr.CodeFor(err) != "fs.companion_failed" { + return err + } + return apperr.New("runtime.companion_incompatible", "runtime", 1, "the installed ti-drive9 companion does not support `ti exec`; update or reinstall ti after a compatible companion release is available") +} + +func companionArgs(opts Options) []string { + args := []string{"exec"} + appendValue := func(name, value string, set bool) { + if set { + args = append(args, name, value) + } + } + appendValue("--workspace", opts.Workspace, opts.WorkspaceSet) + appendValue("--cwd", opts.Cwd, opts.CwdSet) + for _, value := range opts.Environment { + args = append(args, "--env", value) + } + appendValue("--timeout", opts.Timeout, opts.TimeoutSet) + if opts.Fresh { + args = append(args, "--fresh") + } + appendValue("--layer", opts.Layer, opts.LayerSet) + if opts.ReadOnly { + args = append(args, "--read-only") + } + if opts.Verbose { + args = append(args, "--verbose") + } + args = append(args, "--") + return append(args, opts.Command...) +} diff --git a/internal/runtimeexec/service_test.go b/internal/runtimeexec/service_test.go new file mode 100644 index 0000000..d9ea50f --- /dev/null +++ b/internal/runtimeexec/service_test.go @@ -0,0 +1,347 @@ +package runtimeexec + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/tidbcloud/ti-cli/internal/api/endpoints" + "github.com/tidbcloud/ti-cli/internal/apperr" + "github.com/tidbcloud/ti-cli/internal/config" + "github.com/tidbcloud/ti-cli/internal/fs/fscred" +) + +func TestCompanionArgsPreserveRuntimeContract(t *testing.T) { + opts := Options{ + Workspace: ":/repo", WorkspaceSet: true, + Cwd: "web", CwdSet: true, + Environment: []string{"CI=1", "TOKEN"}, + Timeout: "20m", TimeoutSet: true, + Fresh: true, Layer: "tag:agent=one", LayerSet: true, + ReadOnly: true, Verbose: true, + Command: []string{"make", "-j8", "test", "--output", "text"}, + } + want := []string{ + "exec", "--workspace", ":/repo", "--cwd", "web", + "--env", "CI=1", "--env", "TOKEN", "--timeout", "20m", + "--fresh", "--layer", "tag:agent=one", "--read-only", "--verbose", + "--", "make", "-j8", "test", "--output", "text", + } + if got := companionArgs(opts); strings.Join(got, "\x00") != strings.Join(want, "\x00") { + t.Fatalf("companion args = %#v, want %#v", got, want) + } +} + +func TestExecStreamsAndUsesResourceScopedEnvironment(t *testing.T) { + companion, recordPath := buildRuntimeCompanion(t) + home := t.TempDir() + profile := runtimeProfile(home) + t.Setenv("TI_TEST_RUNTIME_RECORD", recordPath) + t.Setenv("TI_TEST_RUNTIME_MODE", "success") + t.Setenv("TI_FS_TOKEN", "ambient-ti-token") + t.Setenv("TIDB_CLOUD_PRIVATE_KEY", "ambient-private") + t.Setenv("DRIVE9_API_KEY", "ambient-drive9-token") + + var stdout, stderr, debug bytes.Buffer + service := testService(home, companion, &stdout, &stderr) + service.Debug = true + service.DebugWriter = &debug + err := service.Exec(context.Background(), Options{ + Profile: profile, Workspace: "repo", WorkspaceSet: true, + Environment: []string{"CI=1"}, Command: []string{"printf", "ok"}, + }) + if err != nil { + t.Fatalf("Exec failed: %v", err) + } + if stdout.String() != "runtime stdout\n" || stderr.String() != "runtime stderr\n" { + t.Fatalf("streams changed or merged: stdout=%q stderr=%q", stdout.String(), stderr.String()) + } + if debug.String() != "ti [DEBUG]: starting Runtime companion\n" { + t.Fatalf("debug output = %q", debug.String()) + } + for _, secret := range []string{"selected-api-key", "ambient-ti-token", "ambient-private", "ambient-drive9-token", "printf", "CI=1"} { + if strings.Contains(debug.String(), secret) { + t.Fatalf("debug output leaked %q: %q", secret, debug.String()) + } + } + + records := readRuntimeRecords(t, recordPath) + if len(records) != 2 || records[0].Kind != "help" || records[1].Kind != "exec" { + t.Fatalf("records = %#v, want one capability check and one exec", records) + } + preflight := records[0] + if preflight.APIKey != "" || preflight.Server != "" || preflight.Region != "" || preflight.TIFSToken != "" || preflight.TiDBPrivate != "" { + t.Fatalf("capability preflight received credentials or remote placement: %#v", preflight) + } + wantHome, err := fscred.CompanionHome(home, profile.Name, profile.FSResourceName) + if err != nil { + t.Fatal(err) + } + execution := records[1] + if execution.Home != wantHome || execution.APIKey != "selected-api-key" || execution.Server != "https://fs.test" || execution.Region != "aws-us-east-1" { + t.Fatalf("execution environment = %#v", execution) + } + if execution.TIFSToken != "" || execution.TiDBPrivate != "" { + t.Fatalf("ambient ti credentials leaked: %#v", execution) + } +} + +func TestExecPreservesTerminalExitAndNeverRetries(t *testing.T) { + for _, test := range []struct { + mode string + code int + }{ + {mode: "nonzero", code: 23}, + {mode: "timeout", code: 124}, + {mode: "canceled", code: 130}, + {mode: "unknown", code: 255}, + } { + t.Run(test.mode, func(t *testing.T) { + companion, recordPath := buildRuntimeCompanion(t) + home := t.TempDir() + t.Setenv("TI_TEST_RUNTIME_RECORD", recordPath) + t.Setenv("TI_TEST_RUNTIME_MODE", test.mode) + var stdout, stderr bytes.Buffer + err := testService(home, companion, &stdout, &stderr).Exec(context.Background(), Options{ + Profile: runtimeProfile(home), Command: []string{"run-once"}, + }) + if got := apperr.ExitCodeFor(err); got != test.code || !apperr.IsSilent(err) { + t.Fatalf("error = %#v, exit=%d silent=%t; want %d and silent", err, got, apperr.IsSilent(err), test.code) + } + records := readRuntimeRecords(t, recordPath) + executions := 0 + for _, record := range records { + if record.Kind == "exec" { + executions++ + } + } + if executions != 1 { + t.Fatalf("execution attempts = %d, want 1; records=%#v", executions, records) + } + }) + } +} + +func TestExecRejectsIncompatibleCompanionBeforeExecution(t *testing.T) { + companion, recordPath := buildRuntimeCompanion(t) + home := t.TempDir() + t.Setenv("TI_TEST_RUNTIME_RECORD", recordPath) + t.Setenv("TI_TEST_RUNTIME_MODE", "incompatible") + err := testService(home, companion, &bytes.Buffer{}, &bytes.Buffer{}).Exec(context.Background(), Options{ + Profile: runtimeProfile(home), Command: []string{"must-not-run"}, + }) + if apperr.CodeFor(err) != "runtime.companion_incompatible" { + t.Fatalf("error = %v, code=%q", err, apperr.CodeFor(err)) + } + records := readRuntimeRecords(t, recordPath) + if len(records) != 1 || records[0].Kind != "help" { + t.Fatalf("incompatible companion started an execution: %#v", records) + } + if records[0].APIKey != "" || records[0].Server != "" || records[0].Region != "" { + t.Fatalf("incompatible companion preflight received credentials or placement: %#v", records[0]) + } +} + +func TestExecCapabilityUsesExitStatusNotHelpText(t *testing.T) { + companion, recordPath := buildRuntimeCompanion(t) + home := t.TempDir() + t.Setenv("TI_TEST_RUNTIME_RECORD", recordPath) + t.Setenv("TI_TEST_RUNTIME_MODE", "alternate-help") + err := testService(home, companion, &bytes.Buffer{}, &bytes.Buffer{}).Exec(context.Background(), Options{ + Profile: runtimeProfile(home), Command: []string{"run-once"}, + }) + if err != nil { + t.Fatalf("successful capability status was rejected because of help prose: %v", err) + } + records := readRuntimeRecords(t, recordPath) + if len(records) != 2 || records[0].Kind != "help" || records[1].Kind != "exec" { + t.Fatalf("records = %#v, want capability gate followed by one execution", records) + } +} + +func TestExecPreservesMissingCompanionError(t *testing.T) { + home := t.TempDir() + t.Setenv("PATH", t.TempDir()) + t.Setenv("TI_DRIVE9_BIN", filepath.Join(t.TempDir(), "missing-ti-drive9")) + err := testService(home, "", &bytes.Buffer{}, &bytes.Buffer{}).Exec(context.Background(), Options{ + Profile: runtimeProfile(home), Command: []string{"must-not-run"}, + }) + if apperr.CodeFor(err) != "fs.companion_missing" || !strings.Contains(apperr.MessageFor(err), "ti requires the Drive9 companion binary") { + t.Fatalf("missing companion error = %#v", err) + } +} + +func TestExecPreservesCancellationDuringCapabilityCheck(t *testing.T) { + companion, recordPath := buildRuntimeCompanion(t) + home := t.TempDir() + t.Setenv("TI_TEST_RUNTIME_RECORD", recordPath) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + err := testService(home, companion, &bytes.Buffer{}, &bytes.Buffer{}).Exec(ctx, Options{ + Profile: runtimeProfile(home), Command: []string{"must-not-run"}, + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context cancellation", err) + } + if _, statErr := os.Stat(recordPath); !os.IsNotExist(statErr) { + t.Fatalf("canceled capability check invoked companion: %v", statErr) + } +} + +func TestExecTreatsStreamFailureAsUnknownWithoutRetry(t *testing.T) { + companion, recordPath := buildRuntimeCompanion(t) + home := t.TempDir() + t.Setenv("TI_TEST_RUNTIME_RECORD", recordPath) + t.Setenv("TI_TEST_RUNTIME_MODE", "success") + service := Service{ + HomeDir: home, CompanionPath: companion, Stdout: failingWriter{}, Stderr: &bytes.Buffer{}, + Resolver: endpoints.Resolver{FSBaseURLs: map[endpoints.ProviderRegion]string{ + {Provider: "aws", Region: "us-east-1"}: "https://fs.test", + }}, + } + err := service.Exec(context.Background(), Options{ + Profile: runtimeProfile(home), Command: []string{"run-once"}, + }) + if apperr.CodeFor(err) != "runtime.outcome_unknown" || apperr.ExitCodeFor(err) != 255 || apperr.IsSilent(err) { + t.Fatalf("stream error = %#v, code=%q exit=%d silent=%t", err, apperr.CodeFor(err), apperr.ExitCodeFor(err), apperr.IsSilent(err)) + } + records := readRuntimeRecords(t, recordPath) + executions := 0 + for _, record := range records { + if record.Kind == "exec" { + executions++ + } + } + if executions != 1 { + t.Fatalf("execution attempts = %d, want 1; records=%#v", executions, records) + } +} + +type failingWriter struct{} + +func (failingWriter) Write([]byte) (int, error) { + return 0, errors.New("test stream failed") +} + +func testService(home, companion string, stdout, stderr *bytes.Buffer) Service { + return Service{ + HomeDir: home, CompanionPath: companion, Stdout: stdout, Stderr: stderr, + Resolver: endpoints.Resolver{FSBaseURLs: map[endpoints.ProviderRegion]string{ + {Provider: "aws", Region: "us-east-1"}: "https://fs.test", + }}, + } +} + +func runtimeProfile(home string) *config.Profile { + return &config.Profile{ + Name: "stage", HomeDir: home, FSResourceName: "tenant-runtime", FSTenantID: "tenant-runtime", + FSPlacementRegionCode: "aws-us-east-1", FSCloudProvider: "aws", FSRegionCode: "us-east-1", FSAPIKey: "selected-api-key", + } +} + +type runtimeRecord struct { + Kind string `json:"kind"` + Args []string `json:"args"` + Home string `json:"home"` + APIKey string `json:"api_key"` + Server string `json:"server"` + Region string `json:"region"` + TIFSToken string `json:"ti_fs_token"` + TiDBPrivate string `json:"tidb_private"` +} + +func readRuntimeRecords(t *testing.T, path string) []runtimeRecord { + t.Helper() + file, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer file.Close() + var records []runtimeRecord + scanner := bufio.NewScanner(file) + for scanner.Scan() { + var record runtimeRecord + if err := json.Unmarshal(scanner.Bytes(), &record); err != nil { + t.Fatal(err) + } + records = append(records, record) + } + if err := scanner.Err(); err != nil { + t.Fatal(err) + } + return records +} + +func buildRuntimeCompanion(t *testing.T) (string, string) { + t.Helper() + dir := t.TempDir() + source := filepath.Join(dir, "main.go") + if err := os.WriteFile(source, []byte(runtimeCompanionSource), 0o600); err != nil { + t.Fatal(err) + } + name := "fake-runtime" + if runtime.GOOS == "windows" { + name += ".exe" + } + binary := filepath.Join(dir, name) + command := exec.Command("go", "build", "-o", binary, source) + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("build fake Runtime companion: %v\n%s", err, output) + } + return binary, filepath.Join(dir, "calls.jsonl") +} + +const runtimeCompanionSource = `package main + +import ( + "encoding/json" + "fmt" + "os" +) + +type record struct { + Kind string ` + "`json:\"kind\"`" + ` + Args []string ` + "`json:\"args\"`" + ` + Home string ` + "`json:\"home\"`" + ` + APIKey string ` + "`json:\"api_key\"`" + ` + Server string ` + "`json:\"server\"`" + ` + Region string ` + "`json:\"region\"`" + ` + TIFSToken string ` + "`json:\"ti_fs_token\"`" + ` + TiDBPrivate string ` + "`json:\"tidb_private\"`" + ` +} + +func main() { + args := os.Args[1:] + kind := "other" + if len(args) >= 2 && args[0] == "exec" && args[1] == "--help" { kind = "help" } + if len(args) > 0 && args[0] == "exec" && kind != "help" { kind = "exec" } + if file, err := os.OpenFile(os.Getenv("TI_TEST_RUNTIME_RECORD"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0600); err == nil { + _ = json.NewEncoder(file).Encode(record{Kind: kind, Args: args, Home: os.Getenv("HOME"), APIKey: os.Getenv("DRIVE9_API_KEY"), Server: os.Getenv("DRIVE9_SERVER"), Region: os.Getenv("DRIVE9_REGION_CODE"), TIFSToken: os.Getenv("TI_FS_TOKEN"), TiDBPrivate: os.Getenv("TIDB_CLOUD_PRIVATE_KEY")}) + _ = file.Close() + } + mode := os.Getenv("TI_TEST_RUNTIME_MODE") + if kind == "help" { + if mode == "incompatible" { fmt.Fprintln(os.Stderr, "drive9: unknown command exec"); os.Exit(2) } + if mode == "alternate-help" { fmt.Fprintln(os.Stderr, "human help may change"); return } + fmt.Fprintln(os.Stderr, "usage: drive9 exec [flags] [--] command [arg...]") + return + } + if kind != "exec" { os.Exit(2) } + fmt.Fprintln(os.Stdout, "runtime stdout") + fmt.Fprintln(os.Stderr, "runtime stderr") + switch mode { + case "nonzero": os.Exit(23) + case "timeout": os.Exit(124) + case "canceled": os.Exit(130) + case "unknown": os.Exit(255) + } +} +` diff --git a/internal/update/update.go b/internal/update/update.go index 1e825e3..85d1dac 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -769,6 +769,10 @@ func validateDrive9Companion(ctx context.Context, path string) error { return incompatibleDrive9CompanionError("missing `drive9 mount --"+flag+"`", mountHelp, nil) } } + + if _, err := drive9HelpOutput(ctx, path, "exec", "--help"); err != nil { + return incompatibleDrive9CompanionError("missing `drive9 exec` Runtime command", "", err) + } return nil } diff --git a/internal/update/update_test.go b/internal/update/update_test.go index c6dd926..8c23122 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -243,6 +243,33 @@ func TestApplyRejectsIncompatibleCompanionBeforeReplacingFiles(t *testing.T) { } } +func TestValidateDrive9CompanionRejectsLegacySurfaceWithoutRuntimeExec(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell companion fixture is Unix-only") + } + path := filepath.Join(t.TempDir(), "drive9") + legacy := `#!/bin/sh +if [ "$1 $2 $3" = "fs layer help" ]; then + echo 'usage: drive9 fs layer ' + exit 0 +fi +if [ "$1 $2" = "mount --help" ]; then + echo ' -layer string' + echo ' -checkpoint string' + exit 0 +fi +echo 'drive9: unknown command exec' >&2 +exit 2 +` + if err := os.WriteFile(path, []byte(legacy), 0o755); err != nil { + t.Fatal(err) + } + err := validateDrive9Companion(context.Background(), path) + if apperr.CodeFor(err) != "update.companion_incompatible" || !strings.Contains(apperr.MessageFor(err), "drive9 exec") { + t.Fatalf("legacy companion error = %#v", err) + } +} + const compatibleDrive9CompanionScript = `#!/bin/sh if [ "$1 $2 $3" = "fs layer help" ]; then echo 'usage: drive9 fs layer ' @@ -253,6 +280,9 @@ if [ "$1 $2" = "mount --help" ]; then echo ' -checkpoint string' exit 0 fi +if [ "$1 $2" = "exec --help" ]; then + exit 0 +fi echo companion ` diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 760fa15..1826453 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -42,6 +42,11 @@ function Assert-CompanionCommandSurface($CompanionPath) { Fail "downloaded ti-drive9 companion is incompatible: missing drive9 mount --$flag" } } + + $null = (& $CompanionPath exec --help 2>&1 | Out-String) + if ($LASTEXITCODE -ne 0) { + Fail "downloaded ti-drive9 companion is incompatible: missing drive9 Runtime exec command; retry after the matching companion release is published" + } } function Resolve-InstallDir { diff --git a/scripts/install.sh b/scripts/install.sh index c124d99..751673c 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -120,6 +120,10 @@ validate_companion() { error "downloaded ti-drive9 companion is incompatible: missing drive9 mount --${flag}" fi done + + if ! "$companion_path" exec --help >/dev/null 2>&1; then + error "downloaded ti-drive9 companion is incompatible: missing drive9 Runtime exec command; retry after the matching companion release is published" + fi } case "$(uname -s)" in From f33d0906287c78d98d631276b313729747719a1c Mon Sep 17 00:00:00 2001 From: qiffang Date: Tue, 20 Oct 2026 12:00:43 +0800 Subject: [PATCH 2/2] fix(exec): preserve cancellation and clarify the Runtime CLI contract --- AGENTS.md | 4 + README.md | 19 +- cmd/ti/main.go | 5 +- docs/pingcap-docs/docs | 2 +- docs/priciples.md | 6 +- docs/releasing.md | 15 ++ docs/spec/{done => }/0036-runtime-exec.md | 37 +++- docs/spec/done/0001-cli-foundation.md | 2 +- e2e/cli_test.go | 4 +- e2e/live_test.go | 2 +- e2e/runtime_signal_unix_test.go | 200 ++++++++++++++++------ e2e/testdata/fake-drive9.go | 24 ++- internal/cli/exec.go | 87 +++------- internal/cli/exec_test.go | 123 ++++++++----- internal/cli/invocation.go | 110 ++++++++++++ internal/cli/root.go | 124 +++----------- internal/cli/root_test.go | 2 +- internal/fswrap/process_unix.go | 19 ++ internal/fswrap/process_unix_test.go | 68 ++++++++ internal/fswrap/process_windows.go | 18 ++ internal/fswrap/runner.go | 84 ++++++++- internal/interrupt/context.go | 68 ++++++++ internal/interrupt/context_test.go | 36 ++++ internal/runtimeexec/service.go | 39 ++--- internal/runtimeexec/service_test.go | 67 ++++++++ 25 files changed, 840 insertions(+), 325 deletions(-) rename docs/spec/{done => }/0036-runtime-exec.md (84%) create mode 100644 internal/cli/invocation.go create mode 100644 internal/fswrap/process_unix.go create mode 100644 internal/fswrap/process_unix_test.go create mode 100644 internal/fswrap/process_windows.go create mode 100644 internal/interrupt/context.go create mode 100644 internal/interrupt/context_test.go diff --git a/AGENTS.md b/AGENTS.md index 1aff151..7092fc1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -520,6 +520,8 @@ Follow these rules unless `docs/priciples.md` is updated: Implemented command behavior: +- `ti exec [flags] [--] command [arg...]`: streamed remote stdout/stderr, exact exit status, explicit Filesystem selection, and no retries; see `docs/spec/0036-runtime-exec.md` for the pending release gate + - `ti` without a command returns `cli.missing_command` with exit code `2` and an AWS-style compact two-level usage synopsis on stderr - `ti configure` @@ -674,6 +676,8 @@ Implemented command behavior: Registered command surface: +- `ti exec [flags] [--] command [arg...]` + - `ti update --check` - `ti update` - `ti db create-db-cluster` diff --git a/README.md b/README.md index 743428a..bf858ab 100644 --- a/README.md +++ b/README.md @@ -179,23 +179,24 @@ Automatic mounting uses FUSE on Linux and WebDAV on macOS and Windows. macOS use Mount commands start the companion runtime in the background, wait until the mount is ready, and then return a structured result. Use `ti fs unmount-file-system` or `ti fs-vault unmount-vault` to end a mount. The public CLI does not expose a foreground mount mode. -Run a command remotely without creating or cleaning up a sandbox yourself: +Run a command in a remote sandbox: ```shell -ti exec -w repo npm test +ti exec --file-system-id -w repo npm test +# With TI_FS_FILE_SYSTEM_ID or TI_FS_TOKEN already set: ti exec -w repo 'npm ci && npm test' ti exec -w repo -e CI=1 -e GITHUB_TOKEN -t 20m -- make -j8 test ``` -`ti exec` requires a `ti-drive9` companion that contains the Runtime command -from Drive9 #1022. The installer and updater reject older companions before -installing either binary. A published ti release must record the exact Drive9 -build and checksums and pass the real `make live-e2e-runtime` release gate; -fake-companion tests or a skipped live test are not release evidence. +The selected Filesystem is available at `/workspace`. Standard output and standard error stream live, and the remote command's exact exit status is preserved. Only files under `/workspace` persist; `--fresh` starts a new sandbox without erasing those files. Remote stdin and interactive terminals are not supported. -`ti exec` mounts the selected Filesystem at `/workspace`, streams stdout and stderr live, and exits with the remote command's status. It uses the same `--file-system-id` / `TI_FS_FILE_SYSTEM_ID`, `--fs-token` / `TI_FS_TOKEN`, profile, and region resolution as other data-plane commands. Inside a matching ti-created FUSE mount, omit `-w`: pending local changes are drained first and the command runs in the corresponding directory. WebDAV mounts are not inferred, so macOS and Windows users normally pass `-w`. +Select the Filesystem with `--file-system-id` / `TI_FS_FILE_SYSTEM_ID` or an explicit `--fs-token` / `TI_FS_TOKEN`, using the same profile and region resolution as other data-plane commands. `-w` selects a working directory within the Filesystem, not an isolation boundary. Inside a matching ti-created FUSE mount, omit `-w` to drain pending local writes and run in the corresponding directory. Explicit `-w` does not drain local writes. WebDAV mounts are not inferred; use `-w` there. -The Runtime flags intentionally match `drive9 exec`: `-w/--workspace`, `-C/--cwd`, repeatable `-e/--env`, `-t/--timeout`, `--fresh`, `--layer`, `--read-only`, and `-v/--verbose`. Flag parsing stops at the remote command; use `--` when the command itself starts with a dash. `--output`, `--query`, and `--dry-run` are not supported because output is a pair of live byte streams. Timeout exits `124`, Ctrl-C cancellation exits `130`, and an unknown outcome exits `255`. Executions are never retried: after `255`, inspect the workspace before deciding whether to run the command again. +Execution flags are `-w/--workspace`, `-C/--cwd`, repeatable `-e/--env`, `-t/--timeout`, `--fresh`, `--layer`, `--read-only`, and `-v/--verbose`. Put them before the remote command. `ti exec help` and `ti exec --help` show local help; `ti exec -w repo help` and `ti exec -- help` run a remote command named `help`. Arguments after the remote command, including `--help`, are passed unchanged. `--output`, `--query`, and `--dry-run` are not supported. + +The first Ctrl-C or SIGTERM requests cancellation and waits for workspace drain. A second interruption detaches; the remote command may still be running. Timeout uses status `124`, cancellation `130`, and an unknown outcome `255`. A remote command can also return any of these statuses, or ti's own error statuses such as `2`, `3`, `4`, and `5`; the exit code alone cannot identify the source. Executions are never retried. After an unknown outcome, inspect the workspace before deciding whether to run the command again. + +Use `ti update` or reinstall ti if the local capability check reports an incompatible companion. Release validation is described in [Releasing](docs/releasing.md). Mount readiness means the mounted path is readable: `ti fs mount-file-system` and `ti fs-vault mount-vault` keep polling the mount path until directory listing succeeds, bounded by `--ready-timeout` (default `30s`). If the mount path never becomes readable in time, the command fails with `fs.mount_ready_timeout`, the background mount is left running, and the mount locator is preserved so the matching unmount command can stop it. diff --git a/cmd/ti/main.go b/cmd/ti/main.go index a605b7c..2b97eb5 100644 --- a/cmd/ti/main.go +++ b/cmd/ti/main.go @@ -5,11 +5,12 @@ import ( "encoding/json" "fmt" "os" - "os/signal" + "syscall" "github.com/tidbcloud/ti-cli/internal/apperr" "github.com/tidbcloud/ti-cli/internal/cli" "github.com/tidbcloud/ti-cli/internal/config/homemigration" + "github.com/tidbcloud/ti-cli/internal/interrupt" "github.com/tidbcloud/ti-cli/internal/version" ) @@ -18,7 +19,7 @@ func main() { runHomeMigration() return } - ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) + ctx, stop := interrupt.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() info := version.Current() diff --git a/docs/pingcap-docs/docs b/docs/pingcap-docs/docs index aef3619..a8e00c3 160000 --- a/docs/pingcap-docs/docs +++ b/docs/pingcap-docs/docs @@ -1 +1 @@ -Subproject commit aef361990dae579cb6f1c9cf0b007fe7f474f348 +Subproject commit a8e00c35f5d3f6e8e5fdf533acbd1d5b060a2bab diff --git a/docs/priciples.md b/docs/priciples.md index 7995500..94fb17f 100644 --- a/docs/priciples.md +++ b/docs/priciples.md @@ -30,7 +30,7 @@ ti [subcommand] - Required flags appear before optional flags in help output. - `ti fs` Unix-style aliases shorten only the command name. Their flags stay long and match the canonical command. - Only `ti configure` may prompt. -- Help works through `ti help`, `ti help`, and `ti help`. +- Help works through `ti help`, `ti help`, and `ti help`. For `ti exec`, only the exact bare `help` alias is local; `ti exec -w repo help` and `ti exec -- help` run a remote command named `help`. - `--version` remains available at every command level. - A command does not infer access mode, resource selection, or mutation intent from SQL or other user content. @@ -185,9 +185,9 @@ Each resource runs the companion with isolated state under: ti supplies a sanitized companion environment containing the resolved server, canonical region, and resource owner token. Inherited `DRIVE9_*` values must not override ti's selection. Users do not edit `~/.drive9` for ti workflows. -`ti exec [flags] [--] command [arg...]` mirrors Drive9 Runtime flags exactly: `-w/--workspace`, `-C/--cwd`, repeatable `-e/--env`, `-t/--timeout`, `--fresh`, `--layer`, `--read-only`, and `-v/--verbose`. Flag parsing stops at the remote command boundary. Runtime stdout and stderr are never buffered or rendered as structured output, executions are never retried, and exit statuses `124`, `130`, and `255` retain timeout, cancellation, and unknown-outcome meanings. +`ti exec [flags] [--] command [arg...]` mirrors Drive9 Runtime flags exactly: `-w/--workspace`, `-C/--cwd`, repeatable `-e/--env`, `-t/--timeout`, `--fresh`, `--layer`, `--read-only`, and `-v/--verbose`. Flag parsing stops at the remote command boundary. Runtime stdout and stderr are never buffered or rendered as structured output, executions are never retried, and Runtime uses exit statuses `124`, `130`, and `255` for timeout, cancellation, and unknown outcomes. Remote command statuses are passed through exactly and can overlap these or ti errors; the status alone does not identify its source. Remote stdin and interactive terminals are not supported. -`ti exec` first runs a credential-free local `ti-drive9 exec --help` capability gate and accepts only exit status zero; human help text is not an API. Only after that gate succeeds may ti load the selected profile, resolve a File System credential, or start a credential-bearing companion command. Installers and the updater enforce the same command gate before replacing installed binaries. +`ti exec` first runs a credential-free local `ti-drive9 exec --help` capability gate and accepts only exit status zero; human help text is not an API. The gate has a ten-second deadline, no stdin or visible output, and a disposable HOME. Cancellation is distinct from incompatibility. Only after that gate succeeds may ti load the selected profile, resolve a File System credential, or start a credential-bearing companion command. Installers and the updater enforce the same command gate before replacing installed binaries. Filesystem resource selection is: diff --git a/docs/releasing.md b/docs/releasing.md index 9eecf1d..fa85b6e 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -77,3 +77,18 @@ gh release view v0.2.8 --repo tidbcloud/ti-cli --json assets --jq '.assets[].nam Expect five ti archives (`ti_darwin_amd64.tar.gz`, `ti_darwin_arm64.tar.gz`, `ti_linux_amd64.tar.gz`, `ti_linux_arm64.tar.gz`, `ti_windows_amd64.zip`), `ti_checksums.txt`, `install.sh`, and `install.ps1`. Check that the release notes match the committed file and the stable release is marked latest. Use the new release notes' version-specific installer commands in a disposable environment, and exercise `ti update --target-version v0.2.8` from the previous release in a separate ti-owned installation. Verify both `ti --version` and the sibling `ti-drive9 version`; a successful ti version check alone does not identify the installed companion. + +## Runtime validation status + +The Runtime source integration and local regression tests do not establish +compatibility with a distributed companion. Spec +[0036-runtime-exec.md](spec/0036-runtime-exec.md) remains pending until the +artifact identity, checksums, and successful live release gates are recorded. +Cancellation and unknown-outcome tests using a controlled companion validate +local signal forwarding and status propagation only. Verify real timeout, +cancellation/drain, and interrupted-response behavior against the Runtime +service before marking that release gate complete. + +Keep Runtime in the aggregate `make live-e2e` gate. Use the focused family +targets for unrelated development; do not turn a skipped Runtime run into a +successful release check. diff --git a/docs/spec/done/0036-runtime-exec.md b/docs/spec/0036-runtime-exec.md similarity index 84% rename from docs/spec/done/0036-runtime-exec.md rename to docs/spec/0036-runtime-exec.md index 3ea8a6d..72d0fd1 100644 --- a/docs/spec/done/0036-runtime-exec.md +++ b/docs/spec/0036-runtime-exec.md @@ -1,5 +1,13 @@ # Runtime Exec +## Verification Status + +Implementation and local regression coverage are complete. Release validation +is pending: record the distributed companion version and checksums, and pass +`make live-e2e-runtime` and the aggregate `make live-e2e` with that artifact. +Fake-companion tests and skipped live tests do not satisfy this gate. Keep this +spec outside `done/` until the release evidence is recorded. + ## Goal Expose Drive9 Runtime through one concise top-level command: @@ -29,7 +37,7 @@ only to `ti exec`. | Flag | Meaning | | --- | --- | -| `-w`, `--workspace PATH` | Drive9 directory to run in; infer an enclosing matching local FUSE mount when omitted, otherwise use the drive root | +| `-w`, `--workspace PATH` | Filesystem directory to run in; infer an enclosing matching local FUSE mount when omitted, otherwise use the drive root | | `-C`, `--cwd DIR` | Subdirectory of the selected workspace | | `-e`, `--env KEY[=VALUE]` | Set a remote variable; a bare key copies the local value; repeatable | | `-t`, `--timeout DURATION` | Caller deadline; the server may impose a shorter limit | @@ -56,7 +64,8 @@ spaces or shell syntax is passed unchanged to Drive9, which runs it through `--output`, `--query`, and `--dry-run` are not supported. Runtime output is a live byte stream, not a structured ti result. `ti exec help` and `ti exec --help` show command help without loading profiles, credentials, settings -beyond the normal help exclusions, or starting the companion. +beyond the normal help exclusions, or starting the companion. The bare `help` alias applies only without exec +flags or `--`; `ti exec -w repo help` and `ti exec -- help` are remote commands. ## Resource And Credential Resolution @@ -64,7 +73,9 @@ Before loading a profile or resolving any credential, run the local `ti-drive9 exec --help` command with a sanitized environment that contains no profile, endpoint, placement, or credential. Exit status zero is the command capability contract; do not parse human help text. An older companion must -fail here before any workspace synchronization or remote request. +fail here before any workspace synchronization or remote request. Use a +disposable HOME, no stdin, no visible output, and a ten-second deadline. +Distinguish interruption, timeout, startup failure, and incompatibility. After the local capability gate, resolve exactly one File System using the existing data-plane contract: @@ -76,8 +87,8 @@ existing data-plane contract: Resolve the credential from explicit `--fs-token`, then `TI_FS_TOKEN`, then the selected resource's local credential. Resolve placement from explicit -`--region`, `TI_REGION_CODE`, then the selected resource. Runtime requires an -Drive9 API-key credential. A delegated Vault token cannot run commands; +`--region`, `TI_REGION_CODE`, then the selected resource. Runtime requires a +Filesystem API-key credential. A delegated Vault token cannot run commands; service authorization remains authoritative for a scoped API key. Run the companion with the existing resource-scoped HOME: @@ -120,25 +131,33 @@ mount retains its resolved layer. Drive9 refuses unverifiable, cross-account, stale, or non-FUSE mounts. In particular, WebDAV is the default on macOS and Windows, so callers there must usually pass `-w`. Explicit `-w` selects the remote directory and does not -upload local changes first. +upload local changes first. `-w` changes the working directory, not the +Filesystem isolation boundary. Only `/workspace` persists; `--fresh` replaces +the sandbox without erasing those files. ## Streams, Exit Status, And Cancellation -- Connect the user's stdin, stdout, and stderr directly to the companion. +- Connect stdout and stderr directly to the companion. The upstream protocol + has no remote stdin channel or interactive terminal support. - Keep stdout and stderr separate and live; do not buffer, combine, decode, query, or re-render remote bytes. - A completed remote command exits with its exact status. Do not add a `ti [ERROR]` line for a nonzero remote status. - Timeout exits `124`, first-interrupt cancellation exits `130`, and unknown outcome exits `255`, using the companion's actionable stderr message. -- Missing/not-executable commands keep shell statuses `127`/`126`. +- Missing/not-executable commands keep shell statuses `127`/`126`. Remote + commands can return any status, including ti and Runtime error codes; a + numeric status alone cannot distinguish their sources. - A busy or not-enabled rejection is a known pre-start failure. Lost, malformed, oversized, contradictory, redirected, or interrupted streams after possible acceptance are `outcome_unknown` and exit `255`. - Never retry or resubmit an execution. The command may already have run. - The companion owns the two-stage interrupt protocol: the first Ctrl-C requests remote cancellation and waits for workspace drain; the second - detaches. The ti parent must not kill the companion on the first interrupt. + detaches. Isolate the companion process group and forward signals exactly + once, including parent-only SIGINT and SIGTERM. A programmatic context + cancellation forwards one interrupt. The ti parent must not kill the + companion on the first interrupt. ## Privacy And Observability diff --git a/docs/spec/done/0001-cli-foundation.md b/docs/spec/done/0001-cli-foundation.md index 0be5400..7693d02 100644 --- a/docs/spec/done/0001-cli-foundation.md +++ b/docs/spec/done/0001-cli-foundation.md @@ -1,6 +1,6 @@ # CLI Foundation -> **Runtime exception:** `0036-runtime-exec.md` adds top-level `ti exec` and +> **Runtime exception:** [`0036-runtime-exec.md`](../0036-runtime-exec.md) adds top-level `ti exec` and > its exact `-w`, `-C`, `-e`, `-t`, and `-v` Drive9 compatibility flags. The > noun-first and long-only rules below remain authoritative for every other > command. diff --git a/e2e/cli_test.go b/e2e/cli_test.go index ec205d1..5866bcc 100644 --- a/e2e/cli_test.go +++ b/e2e/cli_test.go @@ -1320,7 +1320,7 @@ func TestRuntimeExecCommandSurfaceAndFailureContract(t *testing.T) { incompatible := runTIWithInput(t, bin, "", append(baseEnv, "FAKE_DRIVE9_EXEC_MODE=incompatible"), "exec", "must-not-run") incompatible.wantExitCode(1) - incompatible.wantStderrContains("installed ti-drive9 companion does not support `ti exec`") + incompatible.wantStderrContains("installed Runtime companion does not support `ti exec`") incompatible.wantStdoutNotContains("runtime stdout") incompatibleWithoutSelectionRecord := filepath.Join(t.TempDir(), "incompatible-without-selection.jsonl") @@ -1329,7 +1329,7 @@ func TestRuntimeExecCommandSurfaceAndFailureContract(t *testing.T) { "FAKE_DRIVE9_EXEC_MODE=incompatible", "TI_REGION_CODE=aws-us-east-1", }, "exec", "must-not-resolve") incompatibleWithoutSelection.wantExitCode(1) - incompatibleWithoutSelection.wantStderrContains("installed ti-drive9 companion does not support `ti exec`") + incompatibleWithoutSelection.wantStderrContains("installed Runtime companion does not support `ti exec`") incompatibleWithoutSelection.wantStderrNotContains("file system ID is required") incompatibleWithoutSelectionCalls := readFakeDrive9Calls(t, incompatibleWithoutSelectionRecord) if len(incompatibleWithoutSelectionCalls) != 1 || strings.Join(incompatibleWithoutSelectionCalls[0].Args, "\x00") != "exec\x00--help" { diff --git a/e2e/live_test.go b/e2e/live_test.go index b5a168d..794d92e 100644 --- a/e2e/live_test.go +++ b/e2e/live_test.go @@ -206,7 +206,7 @@ func TestLiveRuntimeExec(t *testing.T) { success := runTIWithInput(t, bin, "", env, "exec", "printf 'ti-runtime-stdout\\n'; printf 'ti-runtime-stderr\\n' >&2") success.wantExitCode(0) - if success.stdout != "ti-runtime-stdout\n" || success.stderr != "ti-runtime-stderr\n" { + if success.stdout != "ti-runtime-stdout\n" || !strings.Contains(success.stderr, "ti-runtime-stderr\n") || strings.Contains(success.stderr, "ti-runtime-stdout") { success.fail("live Runtime stdout and stderr changed or merged") } diff --git a/e2e/runtime_signal_unix_test.go b/e2e/runtime_signal_unix_test.go index b71d352..680bd67 100644 --- a/e2e/runtime_signal_unix_test.go +++ b/e2e/runtime_signal_unix_test.go @@ -8,72 +8,166 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "syscall" "testing" "time" ) -func TestRuntimeExecFirstInterruptReachesCompanion(t *testing.T) { +func TestRuntimeExecSignals(t *testing.T) { bin := tiBinary(t) - home := t.TempDir() companion := filepath.Join(t.TempDir(), "ti-drive9") build := exec.Command("go", "build", "-o", companion, "./testdata/fake-drive9.go") - build.Dir = "." if output, err := build.CombinedOutput(); err != nil { - t.Fatalf("build fake Drive9 companion: %v\n%s", err, output) + t.Fatalf("build companion: %v\n%s", err, output) } - - cmd := exec.Command(bin, "exec", "wait") - cmd.Env = append(os.Environ(), - "HOME="+home, - "TI_LOGGING=off", - "TI_DRIVE9_BIN="+companion, - "TI_FS_TOKEN="+drive9TestToken("tenant-runtime-signal"), - "TI_REGION_CODE=aws-us-east-1", - "FAKE_DRIVE9_EXEC_MODE=wait-for-signal", - ) - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - stdoutPipe, err := cmd.StdoutPipe() - if err != nil { - t.Fatal(err) - } - var stderr bytes.Buffer - cmd.Stderr = &stderr - if err := cmd.Start(); err != nil { - t.Fatal(err) + for _, test := range []struct { + name string + signal syscall.Signal + group, twice bool + }{ + {"terminal interrupt", syscall.SIGINT, true, false}, + {"parent interrupt", syscall.SIGINT, false, false}, + {"parent termination", syscall.SIGTERM, false, false}, + {"second interrupt detaches", syscall.SIGINT, false, true}, + {"second terminal interrupt detaches", syscall.SIGINT, true, true}, + } { + t.Run(test.name, func(t *testing.T) { + mode := "wait-for-signal" + if test.twice { + mode = "wait-for-second-signal" + } + cmd := exec.Command(bin, "exec", "wait") + cmd.Env = append(os.Environ(), "HOME="+t.TempDir(), "TI_LOGGING=off", "TI_TELEMETRY=off", "TI_DRIVE9_BIN="+companion, + "TI_FS_TOKEN="+drive9TestToken("tenant-runtime-signal"), "TI_REGION_CODE=aws-us-east-1", "FAKE_DRIVE9_EXEC_MODE="+mode) + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + pipe, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + lines := make(chan string, 16) + go func() { + defer close(lines) + scanner := bufio.NewScanner(pipe) + for scanner.Scan() { + lines <- scanner.Text() + } + }() + childPID := 0 + t.Cleanup(func() { + if childPID != 0 { + _ = syscall.Kill(childPID, syscall.SIGKILL) + } + _ = cmd.Process.Kill() + _ = cmd.Wait() + }) + awaitLine := func(want string) { + t.Helper() + timer := time.NewTimer(10 * time.Second) + defer timer.Stop() + for { + select { + case line, ok := <-lines: + if !ok { + t.Fatalf("companion exited before %q", want) + } + if strings.HasPrefix(line, "runtime pid ") { + childPID, _ = strconv.Atoi(strings.TrimPrefix(line, "runtime pid ")) + } + if line == want { + return + } + case <-timer.C: + t.Fatalf("companion did not reach %q", want) + } + } + } + awaitLine("runtime ready") + pid := cmd.Process.Pid + if test.group { + pid = -pid + } + if err := syscall.Kill(pid, test.signal); err != nil { + t.Fatal(err) + } + if test.twice { + awaitLine("runtime canceling") + if err := syscall.Kill(pid, test.signal); err != nil { + t.Fatal(err) + } + } + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + select { + case err = <-done: + case <-time.After(10 * time.Second): + _ = syscall.Kill(childPID, syscall.SIGKILL) + _ = cmd.Process.Kill() + <-done + t.Fatal("ti did not finish after interrupt") + } + childPID = 0 + exitErr, ok := err.(*exec.ExitError) + if !ok || exitErr.ExitCode() != 130 { + t.Fatalf("exit=%v stderr=%q", err, stderr.String()) + } + want := "runtime canceled" + if test.twice { + want = "runtime detached" + } + if !strings.Contains(stderr.String(), want) || strings.Contains(stderr.String(), "ti [ERROR]") { + t.Fatalf("stderr=%q", stderr.String()) + } + }) } +} - ready := make(chan struct{}) - done := make(chan struct{}) - var stdout bytes.Buffer - go func() { - defer close(done) - scanner := bufio.NewScanner(stdoutPipe) - for scanner.Scan() { - stdout.WriteString(scanner.Text() + "\n") - if scanner.Text() == "runtime ready" { - close(ready) - } - } - }() - select { - case <-ready: - case <-time.After(10 * time.Second): - _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) - _ = cmd.Wait() - t.Fatalf("Runtime companion did not become ready; stdout=%q stderr=%q", stdout.String(), stderr.String()) - } - if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGINT); err != nil { - t.Fatal(err) - } - err = cmd.Wait() - <-done - exitErr, ok := err.(*exec.ExitError) - if !ok || exitErr.ExitCode() != 130 { - t.Fatalf("ti exec after first interrupt = %v; stdout=%q stderr=%q", err, stdout.String(), stderr.String()) +func TestRuntimeCapabilityInterrupt(t *testing.T) { + companion := filepath.Join(t.TempDir(), "ti-drive9") + if output, err := exec.Command("go", "build", "-o", companion, "./testdata/fake-drive9.go").CombinedOutput(); err != nil { + t.Fatalf("build: %v\n%s", err, output) } - if !strings.Contains(stderr.String(), "runtime canceled") || strings.Contains(stderr.String(), "ti [ERROR]") { - t.Fatalf("first interrupt did not preserve companion cancellation: %q", stderr.String()) + for _, group := range []bool{false, true} { + t.Run(strconv.FormatBool(group), func(t *testing.T) { + record := filepath.Join(t.TempDir(), "calls.jsonl") + cmd := exec.Command(tiBinary(t), "exec", "must-not-run") + cmd.Env = append(os.Environ(), "HOME="+t.TempDir(), "TI_LOGGING=off", "TI_TELEMETRY=off", "TI_DRIVE9_BIN="+companion, "FAKE_DRIVE9_RECORD="+record, "FAKE_DRIVE9_EXEC_MODE=slow-help") + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cmd.Process.Kill(); _ = cmd.Wait() }) + deadline := time.After(10 * time.Second) + for { + if data, err := os.ReadFile(record); err == nil && bytes.Contains(data, []byte("--help")) { + break + } + select { + case <-deadline: + t.Fatal("capability probe did not start") + case <-time.After(10 * time.Millisecond): + } + } + pid := cmd.Process.Pid + if group { + pid = -pid + } + if err := syscall.Kill(pid, syscall.SIGINT); err != nil { + t.Fatal(err) + } + err := cmd.Wait() + exitErr, ok := err.(*exec.ExitError) + if !ok || exitErr.ExitCode() != 130 || !strings.Contains(stderr.String(), "interrupted") || strings.Contains(stderr.String(), "does not support") { + t.Fatalf("probe interrupt=%v stderr=%q", err, stderr.String()) + } + }) } } diff --git a/e2e/testdata/fake-drive9.go b/e2e/testdata/fake-drive9.go index ab34854..cc60453 100644 --- a/e2e/testdata/fake-drive9.go +++ b/e2e/testdata/fake-drive9.go @@ -8,6 +8,8 @@ import ( "os/signal" "sort" "strings" + "syscall" + "time" ) type call struct { @@ -53,6 +55,9 @@ func main() { } args := os.Args[1:] if hasPrefix(args, "exec", "--help") { + if os.Getenv("FAKE_DRIVE9_EXEC_MODE") == "slow-help" { + time.Sleep(15 * time.Second) + } if os.Getenv("FAKE_DRIVE9_EXEC_MODE") == "incompatible" { fmt.Fprintln(os.Stderr, "drive9: unknown command exec") os.Exit(2) @@ -64,12 +69,25 @@ func main() { fmt.Fprintln(os.Stdout, "runtime stdout") fmt.Fprintln(os.Stderr, "runtime stderr") switch os.Getenv("FAKE_DRIVE9_EXEC_MODE") { - case "wait-for-signal": - signals := make(chan os.Signal, 1) - signal.Notify(signals, os.Interrupt) + case "wait-for-signal", "wait-for-second-signal": + signals := make(chan os.Signal, 2) + signal.Notify(signals, os.Interrupt, syscall.SIGTERM) defer signal.Stop(signals) + fmt.Fprintf(os.Stdout, "runtime pid %d\n", os.Getpid()) fmt.Fprintln(os.Stdout, "runtime ready") <-signals + if os.Getenv("FAKE_DRIVE9_EXEC_MODE") == "wait-for-second-signal" { + fmt.Fprintln(os.Stdout, "runtime canceling") + <-signals + fmt.Fprintln(os.Stderr, "runtime detached") + os.Exit(130) + } + select { + case <-signals: + fmt.Fprintln(os.Stderr, "duplicate interrupt") + os.Exit(99) + case <-time.After(200 * time.Millisecond): + } fmt.Fprintln(os.Stderr, "runtime canceled") os.Exit(130) case "nonzero": diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 64bfef5..26992e6 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -2,9 +2,9 @@ package cli import ( "fmt" - "strings" "github.com/spf13/cobra" + "github.com/tidbcloud/ti-cli/internal/apperr" "github.com/tidbcloud/ti-cli/internal/runtimeexec" "github.com/tidbcloud/ti-cli/internal/version" @@ -25,13 +25,25 @@ func newExecCommand(info version.Info) *cobra.Command { flags := &execFlags{} cmd := newCommand(commandSpec{ Use: "exec [flags] [--] command [arg...]", - Short: "Run a command in a sandbox with the selected Filesystem at /workspace.", - Long: `Run one command in a remote sandbox with the selected TiDB Cloud Filesystem at /workspace. + Short: "Run a command in a remote sandbox.", + Long: `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. + +Select a Filesystem with --file-system-id, TI_FS_FILE_SYSTEM_ID, or an explicit +FS token. After selecting it, use -w to choose a Filesystem directory. Inside +a matching local FUSE mount, omit -w to drain pending writes and follow the +local directory. Explicit -w does not drain local writes. WebDAV mounts need -w. -Output streams live and the remote command's exit status is preserved. Inside -a matching local FUSE mount, ti uploads pending changes and runs in the same -directory. Elsewhere, use -w to select a remote workspace directory. Runtime -executions are never retried because a lost response may mean the command ran.`, +Examples: + ti exec --file-system-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 + +The last two examples assume a Filesystem is selected through the environment. +Remote stdin and interactive terminals are not supported. Executions are never +retried: a lost response may mean the command ran.`, RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { return apperr.New("runtime.missing_command", "usage", 2, "missing command; see `ti exec --help`") @@ -81,12 +93,13 @@ executions are never retried because a lost response may mean the command ran.`, }) }, }, info) + cmd.Annotations = map[string]string{execCommandAnnotation: "true"} cmd.Args = func(_ *cobra.Command, _ []string) error { return nil } - cmd.Flags().StringVarP(&flags.workspace, "workspace", "w", "", "Drive9 directory to run in; default: the matching local FUSE mount directory, otherwise the drive root.") + cmd.Flags().StringVarP(&flags.workspace, "workspace", "w", "", "Filesystem directory to run in; defaults to the matching FUSE directory or the Filesystem root.") cmd.Flags().StringVarP(&flags.cwd, "cwd", "C", "", "Subdirectory of the selected workspace to run in.") cmd.Flags().StringArrayVarP(&flags.environment, "env", "e", nil, "Set KEY=VALUE remotely, or copy a local KEY; repeatable.") cmd.Flags().StringVarP(&flags.timeout, "timeout", "t", "", "Stop the command after this duration; the server may use a shorter limit.") - cmd.Flags().BoolVar(&flags.fresh, "fresh", false, "Start in a new sandbox instead of reusing a warm sandbox.") + cmd.Flags().BoolVar(&flags.fresh, "fresh", false, "Start a new sandbox; persisted Filesystem data is kept.") cmd.Flags().StringVar(&flags.layer, "layer", "", "Use a LayerFS layer ID, name, or tag:key=value reference.") cmd.Flags().BoolVar(&flags.readOnly, "read-only", false, "Mount the workspace read-only.") cmd.Flags().BoolVarP(&flags.verbose, "verbose", "v", false, "Show the sandbox, working directory, and duration.") @@ -95,59 +108,3 @@ executions are never retried because a lost response may mean the command ran.`, cmd.Flags().SetInterspersed(false) return cmd } - -func isExecInvocation(args []string) bool { - _, command := invocationCommandIndex(args) - return command == "exec" -} - -func invocationCommandIndex(args []string) (int, string) { - valueFlags := map[string]struct{}{ - "profile": {}, "region": {}, "output": {}, "query": {}, - } - for index := 0; index < len(args); index++ { - arg := args[index] - if arg == "--" { - return -1, "" - } - if strings.HasPrefix(arg, "--") { - name, _, hasValue := strings.Cut(strings.TrimPrefix(arg, "--"), "=") - if _, ok := valueFlags[name]; ok && !hasValue && index+1 < len(args) { - index++ - } - continue - } - if strings.HasPrefix(arg, "-") { - return -1, "" - } - return index, arg - } - return -1, "" -} - -func execMetadataArgs(args []string) []string { - execIndex, command := invocationCommandIndex(args) - if command != "exec" { - return args - } - valueFlags := map[string]struct{}{ - "w": {}, "workspace": {}, "C": {}, "cwd": {}, "e": {}, "env": {}, - "t": {}, "timeout": {}, "layer": {}, "file-system-id": {}, "fs-token": {}, - "profile": {}, "region": {}, "output": {}, "query": {}, - } - for index := execIndex + 1; index < len(args); index++ { - arg := args[index] - if arg == "--" { - return args[:index] - } - if !strings.HasPrefix(arg, "-") || arg == "-" { - return args[:index] - } - name := strings.TrimLeft(arg, "-") - name, _, hasValue := strings.Cut(name, "=") - if _, ok := valueFlags[name]; ok && !hasValue && index+1 < len(args) { - index++ - } - } - return args -} diff --git a/internal/cli/exec_test.go b/internal/cli/exec_test.go index 2ee7121..80e4453 100644 --- a/internal/cli/exec_test.go +++ b/internal/cli/exec_test.go @@ -1,60 +1,103 @@ package cli import ( + "bytes" + "context" "slices" + "strings" "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/tidbcloud/ti-cli/internal/version" ) -func TestInvocationCommandIndexSkipsGlobalFlagValues(t *testing.T) { +func TestInvocationMetadataRespectsFlagValuesAndRemoteBoundary(t *testing.T) { tests := []struct { - name string - args []string - index int - command string + args []string + exec, help, debug bool }{ - {name: "plain", args: []string{"exec", "echo"}, index: 0, command: "exec"}, - {name: "global value", args: []string{"--profile", "stage", "--debug", "exec", "echo"}, index: 3, command: "exec"}, - {name: "global attached value", args: []string{"--profile=stage", "exec", "echo"}, index: 1, command: "exec"}, - {name: "flag value named exec", args: []string{"--profile", "exec", "fs", "help"}, index: 2, command: "fs"}, - {name: "invalid short global", args: []string{"-p", "stage", "exec", "echo"}, index: -1}, + {[]string{"--profile", "stage", "--debug", "exec", "echo"}, true, false, true}, + {[]string{"--profile", "exec", "fs", "help"}, false, true, false}, + {[]string{"exec", "-e", "--debug", "echo", "--help"}, true, false, false}, + {[]string{"exec", "-w", "--help", "echo", "--debug"}, true, false, false}, + {[]string{"--profile", "--debug", "exec", "--", "--help"}, true, false, false}, + {[]string{"exec", "-vwrepo", "echo", "--debug"}, true, false, false}, + {[]string{"exec", "-wrepo", "--debug=true", "echo"}, true, false, true}, + {[]string{"exec", "--debug", "--debug=false", "echo"}, true, false, false}, + {[]string{"exec", "--help"}, true, true, false}, + {[]string{"exec", "--help=false", "echo"}, true, false, false}, + {[]string{"exec", "echo", "--help", "--debug"}, true, false, false}, + {[]string{"exec", "--new-value-flag", "--help", "echo"}, true, false, false}, + {[]string{"fs", "list-files", "--path", "--debug"}, false, false, false}, } for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - index, command := invocationCommandIndex(test.args) - if index != test.index || command != test.command { - t.Fatalf("invocationCommandIndex(%#v) = (%d, %q), want (%d, %q)", test.args, index, command, test.index, test.command) + t.Run(strings.Join(test.args, " "), func(t *testing.T) { + root := NewRootCommand(version.Info{}) + findChildCommand(root, "exec").Flags().String("new-value-flag", "", "test registered flag") + call := inspectInvocation(root, test.args) + if call.exec != test.exec || call.help != test.help || call.debug != test.debug { + t.Fatalf("metadata: exec=%t help=%t debug=%t; want %t %t %t", call.exec, call.help, call.debug, test.exec, test.help, test.debug) } }) } } -func TestExecMetadataArgsStopBeforeRemoteCommand(t *testing.T) { - tests := []struct { - name string - args []string - want []string - }{ - { - name: "implicit command boundary", - args: []string{"--profile", "stage", "exec", "-e", "CI=1", "echo", "--debug", "--help"}, - want: []string{"--profile", "stage", "exec", "-e", "CI=1"}, - }, - { - name: "explicit command boundary", - args: []string{"exec", "--fresh", "--", "--debug", "--version"}, - want: []string{"exec", "--fresh"}, - }, - { - name: "non exec unchanged", - args: []string{"fs", "list-files", "--debug"}, - want: []string{"fs", "list-files", "--debug"}, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - if got := execMetadataArgs(test.args); !slices.Equal(got, test.want) { - t.Fatalf("execMetadataArgs(%#v) = %#v, want %#v", test.args, got, test.want) +func TestExecHelpAndRemoteArguments(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("TI_LOGGING", "off") + t.Setenv("TI_TELEMETRY", "off") + for _, test := range []struct{ args, remote []string }{ + {[]string{"exec", "help"}, nil}, + {[]string{"--profile", "stage", "exec", "help"}, nil}, + {[]string{"exec", "--help"}, nil}, + {[]string{"exec", "-w", "repo", "help"}, []string{"help"}}, + {[]string{"exec", "--", "help"}, []string{"help"}}, + {[]string{"exec", "echo", "--help"}, []string{"echo", "--help"}}, + {[]string{"exec", "-vwrepo", "help"}, []string{"help"}}, + {[]string{"exec", "-e", "--debug", "echo", "--version"}, []string{"echo", "--version"}}, + } { + t.Run(strings.Join(test.args, " "), func(t *testing.T) { + root := NewRootCommand(version.Info{}) + var got []string + findChildCommand(root, "exec").RunE = func(_ *cobra.Command, args []string) error { got = args; return nil } + var out, errOut bytes.Buffer + if err := Execute(context.Background(), root, version.Info{}, test.args, &out, &errOut); err != nil { + t.Fatal(err) + } + if !slices.Equal(got, test.remote) { + t.Fatalf("remote args=%q, want %q", got, test.remote) + } + if test.remote == nil && !bytes.Contains(out.Bytes(), []byte("Run a command in a remote sandbox.")) { + t.Fatalf("missing local help: %s", out.String()) } }) } } + +func TestInvocationConsumesEveryRegisteredExecValueFlag(t *testing.T) { + root := NewRootCommand(version.Info{}) + cmd := findChildCommand(root, "exec") + check := func(flag *pflag.Flag) { + if flag.NoOptDefVal != "" { + return + } + for _, value := range []string{"--help", "--debug"} { + args := []string{"exec", "--" + flag.Name, value, "echo"} + call := inspectInvocation(root, args) + if !call.exec || call.help || call.debug { + t.Errorf("flag value interpreted as metadata: %s", flag.Name) + } + if flag.Shorthand != "" { + args[1] = "-" + flag.Shorthand + call = inspectInvocation(root, args) + if !call.exec || call.help || call.debug { + t.Errorf("short flag value interpreted as metadata: %s", flag.Name) + } + } + } + } + cmd.Flags().VisitAll(check) + cmd.InheritedFlags().VisitAll(check) +} diff --git a/internal/cli/invocation.go b/internal/cli/invocation.go new file mode 100644 index 0000000..6255b71 --- /dev/null +++ b/internal/cli/invocation.go @@ -0,0 +1,110 @@ +package cli + +import ( + "strconv" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +const execCommandAnnotation = "ti.exec" + +func isExecCommand(cmd *cobra.Command) bool { + return cmd != nil && cmd.Annotations[execCommandAnnotation] == "true" +} + +type invocation struct { + commandAt int + exec bool + update bool + help bool + debug bool + shortFlag string +} + +// inspectInvocation observes ti's flags without parsing or retaining their +// values. Registered flag definitions determine which arguments consume values; +// the first exec positional (or --) ends inspection of remote arguments. +func inspectInvocation(root *cobra.Command, args []string) invocation { + result := invocation{commandAt: -1} + cmd := root + for i := 0; i < len(args); i++ { + arg := args[i] + if arg == "--" { + break + } + if strings.HasPrefix(arg, "-") && arg != "-" { + long := strings.HasPrefix(arg, "--") + if !long && result.shortFlag == "" { + result.shortFlag = arg + } + name := strings.TrimPrefix(arg, "-") + if long { + name = strings.TrimPrefix(name, "-") + } + for name != "" { + var flag *pflag.Flag + var value string + var attached bool + if long { + name, value, attached = strings.Cut(name, "=") + flag = cmd.Flag(name) + name = "" + } else { + flag = cmd.Flags().ShorthandLookup(name[:1]) + if flag == nil { + flag = cmd.InheritedFlags().ShorthandLookup(name[:1]) + } + name = name[1:] + if flag != nil && len(name) > 0 && (name[0] == '=' || flag.NoOptDefVal == "") { + value, attached, name = strings.TrimPrefix(name, "="), true, "" + } + } + if flag == nil { + // Cobra owns unknown-flag diagnostics. + break + } + if !attached { + value = flag.NoOptDefVal + if value == "" { + i++ // A flag value is never metadata, even if it looks like a flag. + continue + } + } + if enabled, err := strconv.ParseBool(value); err == nil { + switch flag.Name { + case "help", "version": + result.help = result.help || enabled + case "debug": + result.debug = enabled + } + } + } + continue + } + if result.exec { + break + } + child := findChildCommand(cmd, arg) + if child == nil { + break + } + if cmd == root { + result.commandAt = i + result.update = child.Name() == "update" || (child.Name() == "help" && i+1 < len(args) && args[i+1] == "update") + } + cmd = child + result.exec = isExecCommand(child) + result.help = result.help || child.Name() == "help" + } + return result +} + +func normalizeExecHelp(args []string, call invocation) []string { + if call.exec && len(args) == call.commandAt+2 && args[call.commandAt+1] == "help" { + args = append([]string(nil), args...) + args[len(args)-1] = "--help" + } + return args +} diff --git a/internal/cli/root.go b/internal/cli/root.go index db0ff4b..6e106cd 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -33,7 +33,6 @@ const usageRequiredFlagAnnotation = "ti_usage_required" func init() { cobra.AddTemplateFunc("usageSynopsis", usageSynopsis) - cobra.AddTemplateFunc("formattedFlagUsages", formattedFlagUsages) cobra.AddTemplateFunc("formattedFlagUsagesFor", formattedFlagUsagesFor) } @@ -127,7 +126,10 @@ func Execute(ctx context.Context, root *cobra.Command, info version.Info, args [ option(&executeOpts) } } - if !isUpdateInvocation(args) { + call := inspectInvocation(root, args) + args = normalizeExecHelp(args, call) + call = inspectInvocation(root, args) + if !call.update { if err := envcompat.Validate(nil); err != nil { return err } @@ -141,10 +143,10 @@ func Execute(ctx context.Context, root *cobra.Command, info version.Info, args [ } } } - recorder, recordCommand := commandRecorder(args, stderr) + recorder, recordCommand := commandRecorder(call, stderr) ctx = oplog.WithRecorder(ctx, recorder) operationStart := time.Now() - if err := rejectShortFlags(args); err != nil { + if err := rejectShortFlags(call); err != nil { if recordCommand { recorder.Record(ctx, commandEvent(root, root, err, time.Since(operationStart))) } @@ -160,7 +162,7 @@ func Execute(ctx context.Context, root *cobra.Command, info version.Info, args [ Eligible: true, Endpoint: effectiveTelemetryEndpoint(info), Info: info, - Debug: debugRequested(args), + Debug: call.debug, DebugWriter: stderr, }) } @@ -183,7 +185,11 @@ func Execute(ctx context.Context, root *cobra.Command, info version.Info, args [ } func resolvedTelemetryCommand(root *cobra.Command, args []string) *cobra.Command { - if root == nil || isUpdateInvocation(args) || hasTelemetryExclusionFlag(args) { + if root == nil { + return nil + } + call := inspectInvocation(root, args) + if call.update || call.help { return nil } cmd, _, err := root.Find(args) @@ -202,23 +208,6 @@ func hasBusinessSubcommands(cmd *cobra.Command) bool { return false } -func hasTelemetryExclusionFlag(args []string) bool { - args = execMetadataArgs(args) - for _, arg := range args { - if arg == "--" { - return false - } - name, value, hasValue := strings.Cut(strings.TrimPrefix(arg, "--"), "=") - if !strings.HasPrefix(arg, "--") || (name != "help" && name != "version") { - continue - } - if !hasValue || strings.EqualFold(value, "true") || value == "1" { - return true - } - } - return false -} - func effectiveTelemetryEndpoint(info version.Info) string { if endpoint := strings.TrimSpace(info.TelemetryEndpoint); endpoint != "" { return endpoint @@ -274,11 +263,11 @@ func telemetryProfileSource(cmd *cobra.Command) string { return "default" } -func commandRecorder(args []string, stderr io.Writer) (oplog.Recorder, bool) { +func commandRecorder(call invocation, stderr io.Writer) (oplog.Recorder, bool) { disabled := func() (oplog.Recorder, bool) { return oplog.NewRecorder(oplog.Config{Enabled: false}), false } - if isUpdateInvocation(args) { + if call.update { return disabled() } home, err := os.UserHomeDir() @@ -287,7 +276,7 @@ func commandRecorder(args []string, stderr io.Writer) (oplog.Recorder, bool) { } logging, err := settings.ResolveLogging(home, nil) if err != nil { - if debugRequested(args) && stderr != nil { + if call.debug && stderr != nil { _, _ = fmt.Fprintln(stderr, "ti [DEBUG]: operation logging disabled because global settings could not be loaded") } return disabled() @@ -304,55 +293,12 @@ func commandRecorder(args []string, stderr io.Writer) (oplog.Recorder, bool) { return oplog.NewRecorder(cfg), true } -func isUpdateInvocation(args []string) bool { - valueFlags := map[string]struct{}{ - "profile": {}, - "region": {}, - "output": {}, - "query": {}, - } - positionals := make([]string, 0, 2) - for index := 0; index < len(args); index++ { - arg := args[index] - if arg == "--" { - positionals = append(positionals, args[index+1:]...) - break - } - if strings.HasPrefix(arg, "--") { - name, _, hasValue := strings.Cut(strings.TrimPrefix(arg, "--"), "=") - if _, ok := valueFlags[name]; ok && !hasValue && index+1 < len(args) { - index++ - } - continue - } - if strings.HasPrefix(arg, "-") { - continue - } - positionals = append(positionals, arg) - if len(positionals) == 2 { - break - } - } - return len(positionals) > 0 && (positionals[0] == "update" || - (positionals[0] == "help" && len(positionals) > 1 && positionals[1] == "update")) -} - -func debugRequested(args []string) bool { - args = execMetadataArgs(args) - for _, arg := range args { - if arg == "--debug" || strings.EqualFold(arg, "--debug=true") { - return true - } - } - return false -} - func commandEvent(root, cmd *cobra.Command, err error, duration time.Duration) oplog.Event { if cmd == nil { cmd = root } profileName, regionCode := commandProfileSummary(cmd) - if cmd != nil && cmd.Name() == "exec" { + if isExecCommand(cmd) { profileName = "" } return oplog.Event{ @@ -429,22 +375,10 @@ func commandProfileSummary(cmd *cobra.Command) (string, string) { return profileName, configDoc[profileName].RegionCode } -func rejectShortFlags(args []string) error { - if isExecInvocation(args) { - return nil - } - for _, arg := range args { - if arg == "--" { - return nil - } - if len(arg) > 1 && strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") { - return apperr.New( - "cli.short_flag_not_allowed", - "usage", - 2, - fmt.Sprintf("short flags are not supported: %s; use long flags such as --help", arg), - ) - } +func rejectShortFlags(call invocation) error { + if !call.exec && call.shortFlag != "" { + return apperr.New("cli.short_flag_not_allowed", "usage", 2, + fmt.Sprintf("short flags are not supported: %s; use long flags such as --help", call.shortFlag)) } return nil } @@ -831,7 +765,7 @@ func normalizeError(err error) error { func HasNonExecShorthand(cmd *cobra.Command) bool { found := false visitCommands(cmd, func(current *cobra.Command) { - if current.Name() == "exec" { + if isExecCommand(current) { return } for _, set := range []*pflag.FlagSet{ @@ -861,7 +795,7 @@ func installHelpCommands(cmd *cobra.Command, info version.Info) { for _, child := range cmd.Commands() { installHelpCommands(child, info) } - if cmd.Name() == "help" || findChildCommand(cmd, "help") != nil { + if isExecCommand(cmd) || cmd.Name() == "help" || findChildCommand(cmd, "help") != nil { return } @@ -878,7 +812,7 @@ func installHelpCommands(cmd *cobra.Command, info version.Info) { func findChildCommand(cmd *cobra.Command, name string) *cobra.Command { for _, child := range cmd.Commands() { - if child.Name() == name { + if child.Name() == name || child.HasAlias(name) { return child } } @@ -910,7 +844,7 @@ func usageSynopsis(cmd *cobra.Command) string { func usageCommandLine(cmd *cobra.Command) string { line := cmd.CommandPath() - if cmd.Name() == "exec" { + if isExecCommand(cmd) { return line + " [flags] [--] command [arg...]" } if cmd.HasAvailableSubCommands() { @@ -919,10 +853,6 @@ func usageCommandLine(cmd *cobra.Command) string { return line } -func usageFlagLines(flags *pflag.FlagSet) []string { - return usageFlagLinesFor(nil, flags) -} - func usageFlagLinesFor(cmd *cobra.Command, flags *pflag.FlagSet) []string { if flags == nil { return nil @@ -983,10 +913,6 @@ func usageFlagValueType(flag *pflag.Flag) string { } } -func formattedFlagUsages(flags *pflag.FlagSet) string { - return formattedFlagUsagesFor(nil, flags) -} - func formattedFlagUsagesFor(cmd *cobra.Command, flags *pflag.FlagSet) string { if flags == nil { return "" @@ -1027,7 +953,7 @@ func usageFlagVisible(cmd *cobra.Command, flag *pflag.Flag) bool { if flag == nil || flag.Hidden { return false } - return cmd == nil || cmd.Name() != "exec" || (flag.Name != "output" && flag.Name != "query") + return !isExecCommand(cmd) || (flag.Name != "output" && flag.Name != "query") } func formattedFlagDefault(flag *pflag.Flag) string { diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index cb01041..a3a7f24 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -291,7 +291,7 @@ func TestIsUpdateInvocation(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - if got := isUpdateInvocation(test.args); got != test.want { + if got := inspectInvocation(NewRootCommand(version.Info{}), test.args).update; got != test.want { t.Fatalf("isUpdateInvocation(%q) = %t, want %t", test.args, got, test.want) } }) diff --git a/internal/fswrap/process_unix.go b/internal/fswrap/process_unix.go new file mode 100644 index 0000000..78fe870 --- /dev/null +++ b/internal/fswrap/process_unix.go @@ -0,0 +1,19 @@ +//go:build !windows + +package fswrap + +import ( + "os" + "os/exec" + "syscall" +) + +func isolateProcess(cmd *exec.Cmd) { + // Terminal signals reach ti; ti forwards each one exactly once. A separate + // group prevents a single Ctrl-C from also interrupting the child directly. + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} +} + +func interruptProcess(process *os.Process, sig os.Signal) error { + return process.Signal(sig) +} diff --git a/internal/fswrap/process_unix_test.go b/internal/fswrap/process_unix_test.go new file mode 100644 index 0000000..75f6101 --- /dev/null +++ b/internal/fswrap/process_unix_test.go @@ -0,0 +1,68 @@ +//go:build !windows + +package fswrap + +import ( + "context" + "fmt" + "os" + "os/signal" + "sync" + "testing" + "time" + + "github.com/tidbcloud/ti-cli/internal/apperr" +) + +func TestStreamingContextCancellation(t *testing.T) { + if os.Getenv("TI_TEST_STREAMING_CHILD") == "1" { + signals := make(chan os.Signal, 1) + signal.Notify(signals, os.Interrupt) + fmt.Println("ready") + select { + case <-signals: + os.Exit(130) + case <-time.After(20 * time.Second): + os.Exit(99) + } + } + executable, err := os.Executable() + if err != nil { + t.Fatal(err) + } + t.Setenv("TI_TEST_STREAMING_CHILD", "1") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ready := &readyWriter{ready: make(chan struct{})} + runner := Runner{HomeDir: t.TempDir(), CompanionPath: executable, Stdout: ready} + done := make(chan error, 1) + go func() { + done <- runner.RunStreaming(ctx, RunOptions{ResourceName: "test", Args: []string{"-test.run=^TestStreamingContextCancellation$"}}) + }() + select { + case <-ready.ready: + case err := <-done: + t.Fatalf("child exited before ready: %v", err) + case <-time.After(10 * time.Second): + t.Fatal("child did not become ready") + } + cancel() + select { + case err := <-done: + if apperr.ExitCodeFor(err) != 130 || !apperr.IsSilent(err) { + t.Fatalf("cancel=%v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("context cancellation did not reach companion") + } +} + +type readyWriter struct { + ready chan struct{} + once sync.Once +} + +func (w *readyWriter) Write(p []byte) (int, error) { + w.once.Do(func() { close(w.ready) }) + return len(p), nil +} diff --git a/internal/fswrap/process_windows.go b/internal/fswrap/process_windows.go new file mode 100644 index 0000000..7ff2cf4 --- /dev/null +++ b/internal/fswrap/process_windows.go @@ -0,0 +1,18 @@ +package fswrap + +import ( + "os" + "os/exec" + "syscall" + + "golang.org/x/sys/windows" +) + +func isolateProcess(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_NEW_PROCESS_GROUP} +} + +func interruptProcess(process *os.Process, _ os.Signal) error { + // CTRL_BREAK can target a process group and Go delivers it as os.Interrupt. + return windows.GenerateConsoleCtrlEvent(windows.CTRL_BREAK_EVENT, uint32(process.Pid)) +} diff --git a/internal/fswrap/runner.go b/internal/fswrap/runner.go index c34e68f..2441b05 100644 --- a/internal/fswrap/runner.go +++ b/internal/fswrap/runner.go @@ -16,6 +16,7 @@ import ( "github.com/tidbcloud/ti-cli/internal/apperr" "github.com/tidbcloud/ti-cli/internal/config" "github.com/tidbcloud/ti-cli/internal/fs/fscred" + "github.com/tidbcloud/ti-cli/internal/interrupt" ) const ( @@ -108,12 +109,44 @@ func (r Runner) Run(ctx context.Context, opts RunOptions) (Result, error) { return Result{CompanionPath: path, Stdout: stdout.Bytes(), Stderr: stderr.Bytes()}, nil } -// RunStreaming starts one companion command with direct stdin/stdout/stderr -// wiring. It deliberately does not bind the process lifetime to ctx: the -// Runtime companion owns the first-interrupt cancel-and-drain protocol and a -// second interrupt detaches. The parent and child receive terminal signals in -// the same foreground process group, so killing the child on ctx cancellation -// would turn the first Ctrl-C into an unsafe detach. +// Probe runs a local capability check without credentials, stdin, or persistent +// companion state. The caller supplies a bounded context. +func (r Runner) Probe(ctx context.Context, args ...string) error { + if err := ctx.Err(); err != nil { + return err + } + path, err := r.locateCompanion() + if err != nil { + return err + } + home, err := os.MkdirTemp("", "ti-companion-probe-") + if err != nil { + return apperr.Wrap("runtime.probe_failed", "runtime", 1, "cannot prepare the local Runtime capability check", err) + } + defer os.RemoveAll(home) + env, err := r.drive9Env(home, RunOptions{}) + if err != nil { + return err + } + cmd := exec.CommandContext(ctx, path, args...) + isolateProcess(cmd) + cmd.Env = env + err = cmd.Run() + if ctx.Err() != nil { + return ctx.Err() + } + if err == nil { + return nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() > 0 { + return apperr.Wrap("runtime.companion_incompatible", "runtime", 1, "the installed Runtime companion does not support `ti exec`; update or reinstall ti", err) + } + return apperr.Wrap("runtime.companion_failed", "runtime", 1, "cannot run the Runtime companion; update or reinstall ti", err) +} + +// RunStreaming forwards output and signals without killing the companion on +// cancellation. The companion owns cancel-and-drain and second-signal detach. func (r Runner) RunStreaming(ctx context.Context, opts RunOptions) error { select { case <-ctx.Done(): @@ -139,16 +172,20 @@ func (r Runner) RunStreaming(ctx context.Context, opts RunOptions) error { cmd := exec.Command(path, opts.Args...) cmd.Env = env + isolateProcess(cmd) cmd.Stdin = r.Stdin cmd.Stdout = r.Stdout cmd.Stderr = r.Stderr if r.Debug && r.DebugWriter != nil { _, _ = fmt.Fprintln(r.DebugWriter, "ti [DEBUG]: starting Runtime companion") } + if err := ctx.Err(); err != nil { + return err + } if err := cmd.Start(); err != nil { - return apperr.Wrap("runtime.companion_failed", "runtime", 1, "cannot start the Runtime companion; reinstall ti or set TI_DRIVE9_BIN to a compatible drive9 binary", err) + return apperr.Wrap("runtime.companion_failed", "runtime", 1, "cannot start the Runtime companion; update or reinstall ti", err) } - if err := cmd.Wait(); err != nil { + if err := waitStreaming(ctx, cmd); err != nil { var exitErr *exec.ExitError if errors.As(err, &exitErr) && exitErr.ExitCode() > 0 { return apperr.SilentExit("runtime.exec_exit", exitErr.ExitCode()) @@ -158,6 +195,35 @@ func (r Runner) RunStreaming(ctx context.Context, opts RunOptions) error { return nil } +func waitStreaming(ctx context.Context, cmd *exec.Cmd) error { + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + signals, canceled := interrupt.Signals(ctx), ctx.Done() + for { + var sig os.Signal + select { + case err := <-done: + return err + case sig = <-signals: + case <-canceled: + canceled = nil + if interrupt.FromSignal(ctx) { + continue + } + sig = os.Interrupt + } + if err := interruptProcess(cmd.Process, sig); err != nil { + if errors.Is(err, os.ErrProcessDone) { + return <-done + } + // Losing local observation cannot establish that remote work stopped. + _ = cmd.Process.Kill() + <-done + return err + } + } +} + func (r Runner) CompanionInfo(ctx context.Context, profile *config.Profile) (CompanionInfo, error) { result, err := r.Run(ctx, RunOptions{ Profile: profile, @@ -257,7 +323,7 @@ func (r Runner) locateCompanion() (string, error) { return resolved, nil } } - return "", apperr.New("fs.companion_missing", "runtime", 1, "ti requires the Drive9 companion binary; reinstall ti or set TI_DRIVE9_BIN to a compatible drive9 binary") + return "", apperr.New("fs.companion_missing", "runtime", 1, "ti requires the Drive9 companion binary; update or reinstall ti") } func (r Runner) homeDir(opts RunOptions) (string, error) { diff --git a/internal/interrupt/context.go b/internal/interrupt/context.go new file mode 100644 index 0000000..e44e22f --- /dev/null +++ b/internal/interrupt/context.go @@ -0,0 +1,68 @@ +// Package interrupt preserves repeated process signals while canceling work on +// the first signal. Runtime execution forwards the signals to its companion. +package interrupt + +import ( + "context" + "os" + "os/signal" + "sync" +) + +type signalsKey struct{} +type signalCause struct{} + +func (signalCause) Error() string { return "process interrupted" } + +// NotifyContext cancels on the first signal and keeps receiving subsequent +// signals until stop. Callers must call stop to release the signal handler. +func NotifyContext(parent context.Context, sig ...os.Signal) (context.Context, func()) { + incoming := make(chan os.Signal, 2) + signal.Notify(incoming, sig...) + ctx, stop := notifyContext(parent, incoming) + return ctx, func() { + signal.Stop(incoming) + stop() + } +} + +func notifyContext(parent context.Context, incoming <-chan os.Signal) (context.Context, func()) { + outgoing := make(chan os.Signal, 2) + ctx, cancel := context.WithCancelCause(context.WithValue(parent, signalsKey{}, (<-chan os.Signal)(outgoing))) + done := make(chan struct{}) + var once sync.Once + stop := func() { once.Do(func() { close(done); cancel(context.Canceled) }) } + go func() { + for { + select { + case s := <-incoming: + cancel(signalCause{}) + select { + case outgoing <- s: + case <-done: + return + case <-parent.Done(): + return + } + case <-done: + return + case <-parent.Done(): + return + } + } + }() + return ctx, stop +} + +// Signals returns the process signals associated with ctx, or nil for a plain +// context. A consumer must not also forward the first signal via ctx.Done. +func Signals(ctx context.Context) <-chan os.Signal { + signals, _ := ctx.Value(signalsKey{}).(<-chan os.Signal) + return signals +} + +// FromSignal reports whether a process signal caused ctx cancellation. +func FromSignal(ctx context.Context) bool { + _, ok := context.Cause(ctx).(signalCause) + return ok +} diff --git a/internal/interrupt/context_test.go b/internal/interrupt/context_test.go new file mode 100644 index 0000000..7c28fd0 --- /dev/null +++ b/internal/interrupt/context_test.go @@ -0,0 +1,36 @@ +package interrupt + +import ( + "context" + "os" + "testing" + "time" +) + +func TestRepeatedSignalsSurviveContextCancellation(t *testing.T) { + incoming := make(chan os.Signal, 2) + ctx, stop := notifyContext(context.Background(), incoming) + defer stop() + for i := 0; i < 2; i++ { + incoming <- os.Interrupt + select { + case sig := <-Signals(ctx): + if sig != os.Interrupt || !FromSignal(ctx) || ctx.Err() != context.Canceled { + t.Fatalf("signal=%v cause=%v error=%v", sig, context.Cause(ctx), ctx.Err()) + } + case <-time.After(time.Second): + t.Fatal("signal was lost") + } + } + stop() // Cleanup is idempotent. +} + +func TestParentCancellationIsNotAProcessSignal(t *testing.T) { + parent, cancel := context.WithCancel(context.Background()) + ctx, stop := notifyContext(parent, make(chan os.Signal)) + defer stop() + cancel() + if FromSignal(ctx) || ctx.Err() != context.Canceled { + t.Fatalf("cause=%v error=%v", context.Cause(ctx), ctx.Err()) + } +} diff --git a/internal/runtimeexec/service.go b/internal/runtimeexec/service.go index 76b5185..2050d48 100644 --- a/internal/runtimeexec/service.go +++ b/internal/runtimeexec/service.go @@ -4,6 +4,7 @@ import ( "context" "errors" "io" + "time" "github.com/tidbcloud/ti-cli/internal/api/endpoints" "github.com/tidbcloud/ti-cli/internal/apperr" @@ -61,17 +62,8 @@ func (s Service) Exec(ctx context.Context, opts Options) error { // intentionally supplies no profile, endpoint, or credential and accepts the // command's exit status as the contract instead of parsing human help text. func (s Service) Preflight(ctx context.Context) (ReadyService, error) { - runner := fswrap.Runner{ - HomeDir: s.HomeDir, - CompanionPath: s.CompanionPath, - Resolver: s.Resolver, - Stdin: s.Stdin, - Stdout: s.Stdout, - Stderr: s.Stderr, - Debug: s.Debug, - DebugWriter: s.DebugWriter, - } - if err := requireExecCapability(ctx, runner); err != nil { + runner := fswrap.Runner{CompanionPath: s.CompanionPath} + if err := requireExecCapability(ctx, runner, 10*time.Second); err != nil { return ReadyService{}, err } return ReadyService{service: s}, nil @@ -107,24 +99,17 @@ func (r ReadyService) Exec(ctx context.Context, opts Options) error { }) } -func requireExecCapability(ctx context.Context, runner fswrap.Runner) error { - runner.Debug = false - runner.DebugWriter = nil - _, err := runner.Run(ctx, fswrap.RunOptions{ - ResourceName: "runtime-capability", - Args: []string{"exec", "--help"}, - CaptureStdout: true, - }) - if err == nil { - return nil +func requireExecCapability(ctx context.Context, runner fswrap.Runner, timeout time.Duration) error { + probe, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + err := runner.Probe(probe, "exec", "--help") + if ctx.Err() != nil { + return ctx.Err() } - if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - return err - } - if err != nil && apperr.CodeFor(err) != "fs.companion_failed" { - return err + if errors.Is(err, context.DeadlineExceeded) { + return apperr.New("runtime.capability_timeout", "runtime", 1, "the local Runtime capability check timed out; update or reinstall ti if this persists") } - return apperr.New("runtime.companion_incompatible", "runtime", 1, "the installed ti-drive9 companion does not support `ti exec`; update or reinstall ti after a compatible companion release is available") + return err } func companionArgs(opts Options) []string { diff --git a/internal/runtimeexec/service_test.go b/internal/runtimeexec/service_test.go index d9ea50f..eea0c7c 100644 --- a/internal/runtimeexec/service_test.go +++ b/internal/runtimeexec/service_test.go @@ -12,11 +12,13 @@ import ( "runtime" "strings" "testing" + "time" "github.com/tidbcloud/ti-cli/internal/api/endpoints" "github.com/tidbcloud/ti-cli/internal/apperr" "github.com/tidbcloud/ti-cli/internal/config" "github.com/tidbcloud/ti-cli/internal/fs/fscred" + "github.com/tidbcloud/ti-cli/internal/fswrap" ) func TestCompanionArgsPreserveRuntimeContract(t *testing.T) { @@ -305,6 +307,8 @@ import ( "encoding/json" "fmt" "os" + "io" + "time" ) type record struct { @@ -329,6 +333,8 @@ func main() { } mode := os.Getenv("TI_TEST_RUNTIME_MODE") if kind == "help" { + if mode == "slow-help" { time.Sleep(time.Minute) } + if mode == "read-help" { if data, _ := io.ReadAll(os.Stdin); len(data) != 0 { os.Exit(9) } } if mode == "incompatible" { fmt.Fprintln(os.Stderr, "drive9: unknown command exec"); os.Exit(2) } if mode == "alternate-help" { fmt.Fprintln(os.Stderr, "human help may change"); return } fmt.Fprintln(os.Stderr, "usage: drive9 exec [flags] [--] command [arg...]") @@ -345,3 +351,64 @@ func main() { } } ` + +func TestPreflightIsDisposableAndDoesNotReadInput(t *testing.T) { + companion, record := buildRuntimeCompanion(t) + home := t.TempDir() + t.Setenv("TI_TEST_RUNTIME_RECORD", record) + t.Setenv("TI_TEST_RUNTIME_MODE", "read-help") + input := strings.NewReader("input must not reach capability check") + service := testService(home, companion, &bytes.Buffer{}, &bytes.Buffer{}) + service.Stdin = input + if _, err := service.Preflight(context.Background()); err != nil { + t.Fatal(err) + } + if input.Len() == 0 { + t.Fatal("preflight consumed user input") + } + calls := readRuntimeRecords(t, record) + if len(calls) != 1 { + t.Fatalf("calls=%d", len(calls)) + } + if _, err := os.Stat(calls[0].Home); !os.IsNotExist(err) { + t.Fatalf("probe home still exists: %v", err) + } + entries, err := os.ReadDir(home) + if err != nil || len(entries) != 0 { + t.Fatalf("persistent home changed: entries=%v err=%v", entries, err) + } +} + +func TestPreflightCancellationAndDeadline(t *testing.T) { + companion, record := buildRuntimeCompanion(t) + t.Setenv("TI_TEST_RUNTIME_RECORD", record) + t.Setenv("TI_TEST_RUNTIME_MODE", "slow-help") + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan error, 1) + go func() { _, err := (Service{CompanionPath: companion}).Preflight(ctx); done <- err }() + deadline := time.After(10 * time.Second) + for { + if _, err := os.Stat(record); err == nil { + break + } + select { + case <-deadline: + t.Fatal("probe never started") + case <-time.After(10 * time.Millisecond): + } + } + cancel() + select { + case err := <-done: + if !errors.Is(err, context.Canceled) { + t.Fatalf("cancellation=%v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("probe did not cancel") + } + err := requireExecCapability(context.Background(), fswrap.Runner{CompanionPath: companion}, 100*time.Millisecond) + if apperr.CodeFor(err) != "runtime.capability_timeout" { + t.Fatalf("timeout=%v", err) + } +}