From 8f72e34174496b1e51d341f91991bfd84a181d68 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 13:08:29 +0300 Subject: [PATCH 1/2] fix(scaffold): servers a cli app starts never outlive it; redis 8.6.3 uses them io.pilot.redis 8.6.2 leaked on darwin/arm64, darwin/amd64, linux/arm64 and linux/amd64 (harness runs on the published bundles): - redis.start ran `redis-server --daemonize yes`. The server setsid()s out of the adapter's process group and is reparented to pid 1, so it survived the adapter's SIGTERM ("LEFT pid=93603 ppid=1 pgid=93603 redis-server"), the supervisor's stop (SIGKILL of the adapter's pid) and the supervisor's own death, even on Linux where Pdeathsig killed the adapter. Pdeathsig, the supervisor's group stop, reapStale and the child guard cannot reach it. - redis.exec ["redis-server",...,"--daemonize","yes"] leaked the same way, and a relative args[0] ran any host binary through the `redis` dispatcher: ["../"x40+"bin/sh","-c","id -u"] printed "ESCAPED uid=501" (macOS) and "uid=1000" (Linux). - redis.start bound *:port (`CONFIG GET bind` = "* -::*") although the docs say 127.0.0.1, and a second start on a busy port returned exit 0 because the daemonized server forks before it binds. Template: - cli.service: a method that starts a long-running server the adapter owns. The server runs as the adapter's foreground child in its process group, and force_args are appended last, so `--daemonize no` wins over the caller and any config file. The call returns once the server is ready: ready_tcp accepts, or, on a passthrough route, it has stayed up for ready_after. A ready_tcp address that already accepts fails the call before anything starts. A server that is not ready within ready_timeout is stopped, and a process that exits first (--version, a config error) returns its output like a command. How the server is stopped: - on a clean stop, StopServices sends SIGTERM, then SIGKILL after 8s; - while a server runs it holds the child guard (childguard.go), so a SIGKILLed adapter takes it down on every OS; - on Linux the server also gets SIGTERM from the kernel (Pdeathsig) when the adapter dies. - cli.tools: an allowlist for a passthrough route's args[0] (bare names only). - Validation: a passthrough service requires cli.tools and service.tools, and ready_timeout must be below the method timeout. - Documented in docs/CLI-ADAPTER.md. redis submission (8.6.2 -> 8.6.3; the bundled Redis stays 8.6.2): - redis.start is a service: ready_tcp 127.0.0.1:${port}, --bind 127.0.0.1 and force_args --daemonize no. - redis.exec allows only the six bundled tools, and runs redis-server and redis-sentinel as services. Tests: - TestCLIServiceLifetime covers SIGTERM and SIGKILL for enumerated and passthrough services, supervisor death through the mini supervisor, three services stopped by one SIGTERM, one-shot replies, allowlist refusals, a busy port and a server that never gets ready. Each stop must leave an empty process group, and a clean stop must SIGTERM the server. - Mutation checks on darwin/arm64: - With the service not holding the guard, both SIGKILL cases fail ("timed out after 10s waiting for server to die after adapter sigkill"). - Without StopServices, the three SIGTERM cases fail ("server was not stopped with SIGTERM"). - TestCLIServiceConfig covers the validation rules. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/CLI-ADAPTER.md | 54 ++- internal/publish/submission.go | 26 ++ internal/scaffold/config.go | 126 ++++++ internal/scaffold/scaffold.go | 6 +- .../scaffold/templates/client_cli.go.tmpl | 35 ++ internal/scaffold/templates/main.go.tmpl | 8 +- internal/scaffold/templates/service.go.tmpl | 366 ++++++++++++++++++ .../scaffold/templates/svcproc_linux.go.tmpl | 16 + .../scaffold/templates/svcproc_other.go.tmpl | 12 + .../scaffold/zz_cli_service_config_test.go | 65 ++++ .../scaffold/zz_cli_service_lifetime_test.go | 364 +++++++++++++++++ submissions/io.pilot.redis/submission.json | 33 +- 12 files changed, 1096 insertions(+), 15 deletions(-) create mode 100644 internal/scaffold/templates/service.go.tmpl create mode 100644 internal/scaffold/templates/svcproc_linux.go.tmpl create mode 100644 internal/scaffold/templates/svcproc_other.go.tmpl create mode 100644 internal/scaffold/zz_cli_service_config_test.go create mode 100644 internal/scaffold/zz_cli_service_lifetime_test.go diff --git a/docs/CLI-ADAPTER.md b/docs/CLI-ADAPTER.md index 9a62a9c..579bb00 100644 --- a/docs/CLI-ADAPTER.md +++ b/docs/CLI-ADAPTER.md @@ -36,6 +36,55 @@ re-parsed as shell metacharacters. Passthrough is strictly more powerful than an enumerated surface — the caller chooses the subcommand and flags — so reserve it for trusted callers and keep CLI apps `guarded` (below). +A passthrough route whose base command is a dispatcher (a script that runs +`$dir/$1`) can be steered to any host binary by a relative `args[0]` +(`"../../../bin/sh"`). List the tools it may run with `tools:` and every call's +`args[0]` must be exactly one of those bare names: + +```yaml +cli: + passthrough: true + tools: [redis-server, redis-cli, redis-benchmark] +``` + +## Servers the app owns — `cli.service` + +A method that starts a long-running server (a local database, a dev server) +must not fork it into the background: a daemonized server calls `setsid()`, +leaves the adapter's process group and is reparented to init, so it survives +the app's stop, the supervisor's stop and a daemon crash, and nothing ever +stops it. Declare such a method as a service: + +```yaml +- name: redis.start + cli: + args: [redis-server, --port, "${port}", --bind, 127.0.0.1, --dir, "${dir}"] + service: + ready_tcp: "127.0.0.1:${port}" # the call returns once this accepts + ready_timeout: 30s # not ready by then: stopped, call fails + log_file: "${dir}/redis-${port}.log" # its tail is returned on failure + force_args: [--daemonize, "no"] # appended LAST, so it always wins +``` + +The server runs as the adapter's own foreground child, in its process group, +and stays up after the call returns. A clean stop of the app stops it with +SIGTERM (SIGKILL after 8 s); if the app is SIGKILLed, the child guard takes it +down; on Linux it also gets SIGTERM from the kernel when the adapter dies. A +`ready_tcp` address that is already accepting fails the call before anything +starts, and a process that exits before it is ready (`--version`, a config +error) returns its output like a plain command. + +On a passthrough route, `service.tools` names the tools that start a server +(needs `cli.tools`); readiness is `ready_after` (default 1 s) of staying up, +since there are no `${field}` params: + +```yaml +cli: + passthrough: true + tools: [redis-server, redis-cli, redis-sentinel] + service: {tools: [redis-server, redis-sentinel], force_args: [--daemonize, "no"]} +``` + ## Hardening built into the runner The generated `exec.go` is defensive by default: @@ -59,8 +108,9 @@ The generated `exec.go` is defensive by default: taken down by the child guard (`internal/backend/childguard.go`, a copy of the adapter that exists only while a child is in flight). Children stay in the adapter's process group, so a supervisor group stop reaches them too. A - server a CLI deliberately daemonizes into its own session is outside all of - this. + server a CLI daemonizes into its own session (`redis-server --daemonize yes`, + `pg_ctl start`, `mysqld --daemonize`) is outside all of this: declare the + method as a `cli.service` instead (below). - **Staging leaves nothing behind** — native-binary downloads go to `$APP/.staged/tmp`, not the shared `TMPDIR`; a stop during the first-spawn download removes the partial file, and a SIGKILLed start's leftover is swept diff --git a/internal/publish/submission.go b/internal/publish/submission.go index 4df1707..fc125db 100644 --- a/internal/publish/submission.go +++ b/internal/publish/submission.go @@ -231,6 +231,21 @@ type SubCLIRoute struct { Args []string `json:"args"` ParamsAsFlags bool `json:"params_as_flags"` Passthrough bool `json:"passthrough"` + // Tools allowlists a passthrough call's args[0] (see scaffold.CLIRoute.Tools). + Tools []string `json:"tools,omitempty"` + // Service marks a method that starts a long-running server the adapter + // owns and stops with it (see scaffold.CLIService). + Service *SubCLIService `json:"service,omitempty"` +} + +// SubCLIService mirrors scaffold.CLIService. +type SubCLIService struct { + ReadyTCP string `json:"ready_tcp,omitempty"` + ReadyAfter string `json:"ready_after,omitempty"` + ReadyTimeout string `json:"ready_timeout,omitempty"` + LogFile string `json:"log_file,omitempty"` + ForceArgs []string `json:"force_args,omitempty"` + Tools []string `json:"tools,omitempty"` } // SubParam is one structured input parameter (vs the old free-text field). @@ -715,6 +730,17 @@ func (s Submission) ToConfig() *scaffold.Config { Args: m.CLI.Args, ParamsAsFlags: m.CLI.ParamsAsFlags, Passthrough: m.CLI.Passthrough, + Tools: m.CLI.Tools, + } + if sv := m.CLI.Service; sv != nil { + method.CLI.Service = &scaffold.CLIService{ + ReadyTCP: sv.ReadyTCP, + ReadyAfter: sv.ReadyAfter, + ReadyTimeout: sv.ReadyTimeout, + LogFile: sv.LogFile, + ForceArgs: sv.ForceArgs, + Tools: sv.Tools, + } } default: route := &scaffold.HTTPRoute{Verb: orDefault(m.HTTP.Verb, "GET"), Path: m.HTTP.Path, CaptureTo: m.HTTP.CaptureTo} diff --git a/internal/scaffold/config.go b/internal/scaffold/config.go index bf15347..6e423b5 100644 --- a/internal/scaffold/config.go +++ b/internal/scaffold/config.go @@ -905,6 +905,60 @@ type CLIRoute struct { Args []string `yaml:"args"` ParamsAsFlags bool `yaml:"params_as_flags"` Passthrough bool `yaml:"passthrough"` + // Tools (passthrough only) allowlists the first argv element: a call's + // args[0] must be exactly one of these bare names. Without it a + // passthrough argv can name any path the base command will exec (e.g. a + // dispatcher resolving "../../bin/sh"), including a tool that daemonizes + // out of the adapter's reach. + Tools []string `yaml:"tools,omitempty"` + // Service marks the method as starting a long-running server that the + // adapter owns (see CLIService), instead of a run-to-completion command. + Service *CLIService `yaml:"service,omitempty"` +} + +// CLIService declares a method that starts a long-running server. The adapter +// keeps the server as its own foreground child (never daemonized) in its +// process group, returns once the server is ready, and stops it when the +// adapter stops or dies — so no server outlives the app. A server that +// forks itself into the background escapes all of that, which is why +// ForceArgs exists: it is appended last so a last-wins flag such as +// `--daemonize no` overrides anything the caller or a config file set. +type CLIService struct { + // ReadyTCP (enumerated methods only) is a host:port, with ${field} + // placeholders, that accepts a TCP connection once the server is up. The + // call fails fast if something already accepts connections there. + ReadyTCP string `yaml:"ready_tcp,omitempty"` + // ReadyAfter: with no ReadyTCP, the server counts as ready once it has + // stayed up this long (Go duration; default 1s). A process that exits + // sooner (--help, --version, a config error) returns its output like a + // plain command. + ReadyAfter string `yaml:"ready_after,omitempty"` + // ReadyTimeout bounds the wait (default 30s); a server not ready by then + // is stopped and the call fails. Must be below the method's timeout. + ReadyTimeout string `yaml:"ready_timeout,omitempty"` + // LogFile (optional, ${field} placeholders) is where the server writes its + // own log; its tail is returned when the server fails to start. + LogFile string `yaml:"log_file,omitempty"` + // ForceArgs are appended after every other argument. + ForceArgs []string `yaml:"force_args,omitempty"` + // Tools (passthrough only): the args[0] names that start a server; any + // other tool runs as a plain command. Required for a passthrough service. + Tools []string `yaml:"tools,omitempty"` +} + +// ReadyAfterOrDefault / ReadyTimeoutOrDefault feed the generated Spec. +func (s *CLIService) ReadyAfterOrDefault() string { + if s.ReadyAfter == "" { + return "1s" + } + return s.ReadyAfter +} + +func (s *CLIService) ReadyTimeoutOrDefault() string { + if s.ReadyTimeout == "" { + return "30s" + } + return s.ReadyTimeout } // Grants tunes the manifest's declared capabilities. The standard set @@ -1522,6 +1576,78 @@ func (c *Config) validateCLIMethod(i int, m Method) []error { case len(m.CLI.Args) == 0 && !m.CLI.ParamsAsFlags: errs = append(errs, fmt.Errorf("methods[%d] (%s): cli route needs args, params_as_flags, or passthrough", i, m.Name)) } + if m.CLI == nil { + return errs + } + if len(m.CLI.Tools) > 0 && !m.CLI.Passthrough { + errs = append(errs, fmt.Errorf("methods[%d] (%s): cli.tools only applies to a passthrough route", i, m.Name)) + } + for _, t := range m.CLI.Tools { + if t == "" || strings.ContainsAny(t, "/\\") || t == "." || t == ".." { + errs = append(errs, fmt.Errorf("methods[%d] (%s): cli.tools entry %q must be a bare tool name", i, m.Name, t)) + } + } + if sv := m.CLI.Service; sv != nil { + errs = append(errs, validateCLIService(i, m, sv)...) + } + return errs +} + +// validateCLIService checks a cli.service block (see CLIService). +func validateCLIService(i int, m Method, sv *CLIService) []error { + var errs []error + at := fmt.Sprintf("methods[%d] (%s): cli.service", i, m.Name) + durOK := func(name, v string) time.Duration { + if v == "" { + return 0 + } + d, err := time.ParseDuration(v) + if err != nil || d <= 0 { + errs = append(errs, fmt.Errorf("%s.%s %q is not a positive Go duration", at, name, v)) + return 0 + } + return d + } + durOK("ready_after", sv.ReadyAfter) + rt := durOK("ready_timeout", sv.ReadyTimeout) + if rt == 0 { + rt = 30 * time.Second + } + if mt, err := time.ParseDuration(m.TimeoutFor()); err == nil && rt >= mt { + errs = append(errs, fmt.Errorf("%s.ready_timeout %s must be below the method timeout %s", at, rt, mt)) + } + if m.CLI.Passthrough { + if len(sv.Tools) == 0 { + errs = append(errs, fmt.Errorf("%s on a passthrough route needs tools (the args[0] names that start a server)", at)) + } + if sv.ReadyTCP != "" || sv.LogFile != "" { + errs = append(errs, fmt.Errorf("%s: ready_tcp/log_file need ${field} params, which a passthrough route does not have", at)) + } + // Without an allowlist a caller could name the server by another path + // (or wrap it in a shell) and start it daemonized, outside the service. + if len(m.CLI.Tools) == 0 { + errs = append(errs, fmt.Errorf("%s on a passthrough route needs cli.tools (the allowlist of args[0] names)", at)) + } + allowed := map[string]bool{} + for _, t := range m.CLI.Tools { + allowed[t] = true + } + for _, t := range sv.Tools { + if len(m.CLI.Tools) > 0 && !allowed[t] { + errs = append(errs, fmt.Errorf("%s.tools entry %q is not in cli.tools", at, t)) + } + } + } else { + if len(sv.Tools) > 0 { + errs = append(errs, fmt.Errorf("%s.tools only applies to a passthrough route", at)) + } + if sv.ReadyTCP != "" { + host, port, err := net.SplitHostPort(sv.ReadyTCP) + if err != nil || host == "" || port == "" { + errs = append(errs, fmt.Errorf("%s.ready_tcp %q must be host:port (placeholders allowed)", at, sv.ReadyTCP)) + } + } + } return errs } diff --git a/internal/scaffold/scaffold.go b/internal/scaffold/scaffold.go index 22abf52..a4d218e 100644 --- a/internal/scaffold/scaffold.go +++ b/internal/scaffold/scaffold.go @@ -23,12 +23,16 @@ type file struct { // childProcFiles are the per-OS process attributes the exec runner applies to // every CLI child (Linux: parent-death signal), plus the guard that takes an -// in-flight child down with a SIGKILLed adapter; emitted with client_cli.go.tmpl. +// in-flight child down with a SIGKILLed adapter, plus the servers cli.service +// methods start and own (service.go, svcproc_*.go); emitted with client_cli.go.tmpl. func childProcFiles() []file { return []file{ {filepath.Join("internal", "backend", "childproc_linux.go"), "childproc_linux.go.tmpl"}, {filepath.Join("internal", "backend", "childproc_other.go"), "childproc_other.go.tmpl"}, {filepath.Join("internal", "backend", "childguard.go"), "childguard.go.tmpl"}, + {filepath.Join("internal", "backend", "service.go"), "service.go.tmpl"}, + {filepath.Join("internal", "backend", "svcproc_linux.go"), "svcproc_linux.go.tmpl"}, + {filepath.Join("internal", "backend", "svcproc_other.go"), "svcproc_other.go.tmpl"}, } } diff --git a/internal/scaffold/templates/client_cli.go.tmpl b/internal/scaffold/templates/client_cli.go.tmpl index c6f64c2..e85d902 100644 --- a/internal/scaffold/templates/client_cli.go.tmpl +++ b/internal/scaffold/templates/client_cli.go.tmpl @@ -70,6 +70,13 @@ type Spec struct { Args []string // may contain ${field} placeholders, substituted from params ParamsAsFlags bool // if true, append --key value for each payload field (sorted) Passthrough bool // if true, ignore Args; argv comes from the call's "args" array + // Tools (passthrough) allowlists the call's args[0]: it must be exactly one + // of these bare names, so a passthrough argv cannot name an arbitrary path + // for the base command to run (see checkTool). + Tools []string + // Service marks a method that starts a long-running server the adapter + // owns (service.go); nil for a run-to-completion command. + Service *ServiceSpec } // Call carries one invocation's dynamic inputs, decoded from the IPC payload. @@ -97,6 +104,9 @@ func (r *Runner) Run(ctx context.Context, spec Spec, call Call) (json.RawMessage args := append([]string(nil), r.base[1:]...) if spec.Passthrough { + if err := checkTool(spec.Tools, call.Args); err != nil { + return nil, err + } args = append(args, call.Args...) } else { for _, a := range spec.Args { @@ -113,6 +123,10 @@ func (r *Runner) Run(ctx context.Context, spec Spec, call Call) (json.RawMessage } } + if sv := spec.Service; sv != nil && sv.appliesTo(spec, call) { + return r.startService(ctx, sv, args, call) + } + cmd := exec.CommandContext(ctx, r.base[0], args...) cmd.Env = r.childEnv() // On timeout/cancel, ask the CLI to stop (SIGTERM) before exec's WaitDelay @@ -188,6 +202,27 @@ func (r *Runner) childEnv() []string { return env } +// checkTool enforces a passthrough route's tools allowlist (no-op without one). +// A dispatcher base command (e.g. a `redis` script that runs "$dir/$1") would +// otherwise resolve "../../bin/sh" and run any host binary. +func checkTool(tools, args []string) error { + if len(tools) == 0 { + return nil + } + if len(args) > 0 { + for _, t := range tools { + if args[0] == t { + return nil + } + } + } + got := "(none)" + if len(args) > 0 { + got = fmt.Sprintf("%q", args[0]) + } + return fmt.Errorf("backend: args[0] must be one of %s; got %s", strings.Join(tools, ", "), got) +} + // substitute replaces every ${field} token in s with params[field]. An // unresolved token is an error rather than a silently-empty arg, so a missing // required param fails loudly instead of mis-invoking the CLI. diff --git a/internal/scaffold/templates/main.go.tmpl b/internal/scaffold/templates/main.go.tmpl index 7180ac4..3874e06 100644 --- a/internal/scaffold/templates/main.go.tmpl +++ b/internal/scaffold/templates/main.go.tmpl @@ -166,6 +166,10 @@ func main() { serveErr := serve(ctx, *socket, d) {{- if or (eq .Backend.Type "cli") (eq .Backend.Type "hybrid")}} + // Servers started by cli.service methods run until the app stops: stop + // them now (SIGTERM, then SIGKILL after a grace period). Before the guard + // is closed, since a running server holds it. + backend.StopServices() backend.CloseChildGuard(2 * time.Second) {{- end}} if serveErr != nil { @@ -729,7 +733,7 @@ func jsonScalar(v json.RawMessage) string { func registerHandlers(d *ipc.Dispatcher, r *backend.Runner, c *backend.CloudClient, version, backendURL string) { {{- range .Methods}} {{- if .CLI}} - d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} (local) + d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{if .CLI.Tools}}, Tools: {{printf "%#v" .CLI.Tools}}{{end}}{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}}{{with .CLI.Service}}, Service: &backend.ServiceSpec{ReadyTCP: {{printf "%q" .ReadyTCP}}, ReadyAfter: dur({{printf "%q" .ReadyAfterOrDefault}}), ReadyTimeout: dur({{printf "%q" .ReadyTimeoutOrDefault}}), LogFile: {{printf "%q" .LogFile}}, ForceArgs: {{printf "%#v" .ForceArgs}}, Tools: {{printf "%#v" .Tools}}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} (local) {{- else}} d.Register("{{.Name}}", cloudHandler(c, "{{.HTTP.Verb}}", "{{.HTTP.Path}}", {{if eq .HTTP.Path $.ProvisionPath}}true{{else}}false{{end}}, dur("{{.TimeoutFor}}"))) // {{.Duration}} (cloud) {{- end}} @@ -823,7 +827,7 @@ func decodeCall(spec backend.Spec, raw json.RawMessage) (backend.Call, error) { {{- else}} func registerHandlers(d *ipc.Dispatcher, r *backend.Runner, version string) { {{- range .Methods}} - d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} + d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{if .CLI.Tools}}, Tools: {{printf "%#v" .CLI.Tools}}{{end}}{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}}{{with .CLI.Service}}, Service: &backend.ServiceSpec{ReadyTCP: {{printf "%q" .ReadyTCP}}, ReadyAfter: dur({{printf "%q" .ReadyAfterOrDefault}}), ReadyTimeout: dur({{printf "%q" .ReadyTimeoutOrDefault}}), LogFile: {{printf "%q" .LogFile}}, ForceArgs: {{printf "%#v" .ForceArgs}}, Tools: {{printf "%#v" .Tools}}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} {{- end}} d.Register("{{.Namespace}}.help", helpHandler(version)) } diff --git a/internal/scaffold/templates/service.go.tmpl b/internal/scaffold/templates/service.go.tmpl new file mode 100644 index 0000000..d036bb0 --- /dev/null +++ b/internal/scaffold/templates/service.go.tmpl @@ -0,0 +1,366 @@ +// Managed servers for the {{.ID}} cli runner. GENERATED by pilot-app; edit +// pilot.app.yaml and re-generate. +// +// A cli.service method starts a long-running server that the adapter owns, and +// nothing it starts may outlive the adapter. A server that forks itself into +// the background (redis-server --daemonize yes, pg_ctl start, mysqld +// --daemonize) calls setsid(): it leaves the adapter's process group and is +// reparented to init, out of reach of the adapter, of Pdeathsig, of the +// supervisor's group stop and of the child guard. Found with io.pilot.redis +// 8.6.2: redis.start ran `redis-server --daemonize yes` and the server +// survived the adapter's SIGTERM, the supervisor's SIGKILL of the adapter and +// the supervisor's own death, on Linux and macOS. +// +// So a service runs as the adapter's own foreground child, in its process +// group, with cli.service.force_args appended last (e.g. --daemonize no, which +// wins over anything the caller or a config file set). The call returns once +// the server is ready. Then: +// - a clean stop (SIGTERM, or the parent-death watch in main) runs +// StopServices: SIGTERM, then SIGKILL after serviceStopGrace; +// - while a service runs it holds the child guard (childguard.go), so an +// adapter that is SIGKILLed takes the server down with it on every OS; +// - on Linux the server also gets SIGTERM from the kernel when the adapter +// dies (Pdeathsig, svcproc_linux.go). +// The passthrough tools allowlist (checkTool in exec.go) is what stops a +// caller from naming the server by another path, or a shell, to start it +// daemonized anyway. +package backend + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "log" + "net" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "syscall" + "time" +) + +const ( + // serviceStopGrace: SIGTERM → SIGKILL for a service being stopped. + serviceStopGrace = 8 * time.Second + // serviceTailBytes of a service's stdout+stderr are kept for error replies. + serviceTailBytes = 16 << 10 + // logTailBytes of a service's own log file are returned when it fails. + logTailBytes = 4 << 10 +) + +// ServiceSpec is a method's cli.service block, baked in at generation time. +type ServiceSpec struct { + ReadyTCP string // host:port (${field}) that accepts once ready; "" = use ReadyAfter + ReadyAfter time.Duration // no ReadyTCP: ready once the process has stayed up this long + ReadyTimeout time.Duration // stop it and fail the call if not ready by then + LogFile string // the server's own log (${field}); its tail is returned on failure + ForceArgs []string // appended after every other argument (last-wins flags) + Tools []string // passthrough: the args[0] names that start a server +} + +// appliesTo reports whether this call starts a server. Every call of an +// enumerated method does; a passthrough call does when its tool is one of +// Tools (compared by base name, so a path spelling cannot dodge it). +func (s *ServiceSpec) appliesTo(spec Spec, call Call) bool { + if !spec.Passthrough { + return true + } + if len(call.Args) == 0 { + return false + } + tool := filepath.Base(call.Args[0]) + for _, t := range s.Tools { + if tool == t { + return true + } + } + return false +} + +type service struct { + name string + cmd *exec.Cmd + out *tailBuffer + done chan struct{} // closed once Wait has returned + err error // Wait's result, valid after done +} + +var ( + svcMu sync.Mutex + services = map[*service]struct{}{} + svcClosed bool // set by StopServices: no new service may start +) + +// startService runs one service call: start the server, wait until it is +// ready (or exits, or times out), and leave it running under the adapter. +func (r *Runner) startService(ctx context.Context, sv *ServiceSpec, args []string, call Call) (json.RawMessage, error) { + var readyAddr, logFile string + if sv.ReadyTCP != "" { + a, err := substitute(sv.ReadyTCP, call.Params) + if err != nil { + return nil, err + } + readyAddr = a + // Something already answering there would make the readiness probe + // report success for a server that is about to fail to bind. + if dialOK(readyAddr, 300*time.Millisecond) { + return nil, fmt.Errorf("backend: %s is already accepting connections — a server is already running there; stop it or use another port", readyAddr) + } + } + if sv.LogFile != "" { + if lf, err := substitute(sv.LogFile, call.Params); err == nil { + logFile = lf + } + } + argv := append(append([]string(nil), args...), sv.ForceArgs...) + name := filepath.Base(r.base[0]) + if len(argv) > 0 { + name = filepath.Base(argv[0]) + } + + s := &service{name: name, out: &tailBuffer{limit: serviceTailBytes}, done: make(chan struct{})} + cmd := exec.Command(r.base[0], argv...) + cmd.Env = r.childEnv() + cmd.Stdout, cmd.Stderr = s.out, s.out + cmd.WaitDelay = 2 * time.Second // don't wait forever on pipes a grandchild inherited + cmd.SysProcAttr = serviceSysProcAttr() + s.cmd = cmd + + started := make(chan error, 1) + go func() { + // Pdeathsig (Linux) fires when the thread that forked the server + // exits, so that thread stays pinned for the server's whole life. + unlock := lockForChild() + defer unlock() + svcMu.Lock() + if svcClosed { + svcMu.Unlock() + started <- errors.New("adapter is shutting down") + return + } + // The guard is held for as long as the server runs: if the adapter is + // SIGKILLed, it takes the adapter's process group down, server included. + guard.acquire() + err := cmd.Start() + if err == nil { + services[s] = struct{}{} + } else { + guard.release() + } + svcMu.Unlock() + started <- err + if err != nil { + return + } + s.err = cmd.Wait() + svcMu.Lock() + delete(services, s) + svcMu.Unlock() + // Released before done is closed, so a shutdown that waited for done + // finds the guard idle and can retire it (CloseChildGuard). + guard.release() + close(s.done) + }() + if err := <-started; err != nil { + return nil, fmt.Errorf("backend: start %s: %w", name, err) + } + + t0 := time.Now() + deadline := time.NewTimer(sv.ReadyTimeout) + defer deadline.Stop() + tick := time.NewTicker(50 * time.Millisecond) + defer tick.Stop() + for { + select { + case <-s.done: + // Exited before it was ready: a one-shot invocation (--help, + // --version) or a failed start. Either way, a plain CLI result. + return exitedResult(s, logFile) + case <-ctx.Done(): + s.stop(serviceStopGrace) + return nil, fmt.Errorf("backend: %s: %w before it was ready; stopped it%s", name, ctx.Err(), failureDetail(s, logFile)) + case <-deadline.C: + s.stop(serviceStopGrace) + return nil, fmt.Errorf("backend: %s was not ready within %s; stopped it%s", name, sv.ReadyTimeout, failureDetail(s, logFile)) + case <-tick.C: + if readyAddr != "" { + if !dialOK(readyAddr, 250*time.Millisecond) { + continue + } + } else if time.Since(t0) < sv.ReadyAfter { + continue + } + select { + case <-s.done: + return exitedResult(s, logFile) + default: + } + msg := fmt.Sprintf("%s started (pid %d)", name, cmd.Process.Pid) + if readyAddr != "" { + msg += ", accepting connections on " + readyAddr + } + msg += "; it runs under this app and stops when the app stops" + out, _ := json.Marshal(map[string]any{ + "stdout": msg, + "stderr": "", + "exit": 0, + "truncated": false, + "pid": cmd.Process.Pid, + }) + return out, nil + } + } +} + +// stop SIGTERMs the service, then SIGKILLs it after grace, and waits for it. +func (s *service) stop(grace time.Duration) { + _ = s.cmd.Process.Signal(syscall.SIGTERM) + select { + case <-s.done: + return + case <-time.After(grace): + } + _ = s.cmd.Process.Kill() + select { + case <-s.done: + case <-time.After(2 * time.Second): + } +} + +// StopServices stops every running service (SIGTERM, SIGKILL after +// serviceStopGrace) and refuses new ones. Call it on shutdown, after the serve +// loop returns. +func StopServices() { + svcMu.Lock() + svcClosed = true + list := make([]*service, 0, len(services)) + for s := range services { + list = append(list, s) + } + svcMu.Unlock() + if len(list) == 0 { + return + } + for _, s := range list { + _ = s.cmd.Process.Signal(syscall.SIGTERM) + } + deadline := time.After(serviceStopGrace) +wait: + for _, s := range list { + select { + case <-s.done: + case <-deadline: + break wait + } + } + for _, s := range list { + select { + case <-s.done: + default: + log.Printf("backend: service %s (pid %d) ignored SIGTERM for %s; killing it", s.name, s.cmd.Process.Pid, serviceStopGrace) + _ = s.cmd.Process.Kill() + select { + case <-s.done: + case <-time.After(2 * time.Second): + } + } + } +} + +func exitedResult(s *service, logFile string) (json.RawMessage, error) { + code := 0 + if s.err != nil { + var ee *exec.ExitError + if !errors.As(s.err, &ee) { + return nil, fmt.Errorf("backend: %s: %w: %s", s.name, s.err, strings.TrimSpace(s.out.String())) + } + code = ee.ExitCode() + } + res := map[string]any{ + "stdout": strings.TrimSpace(s.out.String()), + "stderr": "", + "exit": code, + "truncated": s.out.dropped(), + } + if code != 0 && logFile != "" { + if t := fileTail(logFile, logTailBytes); t != "" { + res["stderr"] = t + } + } + out, _ := json.Marshal(res) + return out, nil +} + +func failureDetail(s *service, logFile string) string { + var parts []string + if o := strings.TrimSpace(s.out.String()); o != "" { + parts = append(parts, "output: "+o) + } + if logFile != "" { + if t := fileTail(logFile, logTailBytes); t != "" { + parts = append(parts, "log "+logFile+": "+t) + } + } + if len(parts) == 0 { + return "" + } + return " — " + strings.Join(parts, "; ") +} + +func fileTail(path string, n int64) string { + f, err := os.Open(path) + if err != nil { + return "" + } + defer f.Close() + if st, err := f.Stat(); err == nil && st.Size() > n { + _, _ = f.Seek(st.Size()-n, io.SeekStart) + } + b, _ := io.ReadAll(io.LimitReader(f, n)) + return strings.TrimSpace(string(b)) +} + +func dialOK(addr string, timeout time.Duration) bool { + c, err := net.DialTimeout("tcp", addr, timeout) + if err != nil { + return false + } + _ = c.Close() + return true +} + +// tailBuffer keeps the last limit bytes written to it; it never blocks the +// writer, so a chatty server cannot stall on a full pipe or grow memory. +type tailBuffer struct { + mu sync.Mutex + buf []byte + limit int + cut bool +} + +func (t *tailBuffer) Write(p []byte) (int, error) { + t.mu.Lock() + defer t.mu.Unlock() + t.buf = append(t.buf, p...) + if over := len(t.buf) - t.limit; over > 0 { + t.buf = append(t.buf[:0], t.buf[over:]...) + t.cut = true + } + return len(p), nil +} + +func (t *tailBuffer) String() string { + t.mu.Lock() + defer t.mu.Unlock() + return string(t.buf) +} + +func (t *tailBuffer) dropped() bool { + t.mu.Lock() + defer t.mu.Unlock() + return t.cut +} diff --git a/internal/scaffold/templates/svcproc_linux.go.tmpl b/internal/scaffold/templates/svcproc_linux.go.tmpl new file mode 100644 index 0000000..22235f3 --- /dev/null +++ b/internal/scaffold/templates/svcproc_linux.go.tmpl @@ -0,0 +1,16 @@ +//go:build linux + +// Service lifetime for the {{.ID}} cli runner (Linux). GENERATED by pilot-app; +// edit pilot.app.yaml and re-generate. +package backend + +import "syscall" + +// serviceSysProcAttr: a server started by a cli.service method gets SIGTERM — +// a clean shutdown, not SIGKILL — when the adapter thread that started it dies, +// including when the adapter is SIGKILLed or killed by its own Pdeathsig. It +// stays in the adapter's process group so a supervisor group stop and the +// child guard (childguard.go) also reach it. +func serviceSysProcAttr() *syscall.SysProcAttr { + return &syscall.SysProcAttr{Pdeathsig: syscall.SIGTERM} +} diff --git a/internal/scaffold/templates/svcproc_other.go.tmpl b/internal/scaffold/templates/svcproc_other.go.tmpl new file mode 100644 index 0000000..70ed4b4 --- /dev/null +++ b/internal/scaffold/templates/svcproc_other.go.tmpl @@ -0,0 +1,12 @@ +//go:build !linux + +// Service lifetime for the {{.ID}} cli runner (non-Linux). GENERATED by +// pilot-app; edit pilot.app.yaml and re-generate. +package backend + +import "syscall" + +// serviceSysProcAttr: there is no parent-death signal outside Linux. The +// server stays in the adapter's process group; StopServices stops it on a clean +// shutdown and the child guard (childguard.go) when the adapter is killed. +func serviceSysProcAttr() *syscall.SysProcAttr { return nil } diff --git a/internal/scaffold/zz_cli_service_config_test.go b/internal/scaffold/zz_cli_service_config_test.go new file mode 100644 index 0000000..f451fd8 --- /dev/null +++ b/internal/scaffold/zz_cli_service_config_test.go @@ -0,0 +1,65 @@ +package scaffold + +import ( + "strings" + "testing" +) + +// TestCLIServiceConfig: cli.tools / cli.service are validated so a spec cannot +// declare a service a caller could still start daemonized (no tools allowlist), +// or a readiness wait the method timeout would cut short. +func TestCLIServiceConfig(t *testing.T) { + base := ` +id: io.pilot.svc +app_version: 0.1.0 +description: "svc" +namespace: svc +backend: + type: cli + command: ["svc"] +methods: + - name: svc.m + summary: "m" +` + cases := []struct { + name, method, want string + }{ + {"ok enumerated", ` cli: {args: ["serve", "${port}"], service: {ready_tcp: "127.0.0.1:${port}", force_args: ["--daemonize", "no"]}}`, ""}, + {"ok passthrough", ` cli: {passthrough: true, tools: ["a", "b"], service: {tools: ["a"], ready_after: "1s"}}`, ""}, + {"tools on enumerated", ` cli: {args: ["x"], tools: ["a"]}`, "cli.tools only applies to a passthrough route"}, + {"tool with a path", ` cli: {passthrough: true, tools: ["../bin/sh"]}`, "must be a bare tool name"}, + {"passthrough service without service.tools", ` cli: {passthrough: true, tools: ["a"], service: {ready_after: "1s"}}`, "needs tools"}, + {"passthrough service without cli.tools", ` cli: {passthrough: true, service: {tools: ["a"]}}`, "needs cli.tools"}, + {"service tool outside cli.tools", ` cli: {passthrough: true, tools: ["a"], service: {tools: ["b"]}}`, "is not in cli.tools"}, + {"ready_tcp on passthrough", ` cli: {passthrough: true, tools: ["a"], service: {tools: ["a"], ready_tcp: "127.0.0.1:1"}}`, "ready_tcp/log_file need"}, + {"bad ready_tcp", ` cli: {args: ["x"], service: {ready_tcp: "nohostport"}}`, "must be host:port"}, + {"bad duration", ` cli: {args: ["x"], service: {ready_after: "soon"}}`, "not a positive Go duration"}, + {"ready_timeout past method timeout", ` cli: {args: ["x"], service: {ready_timeout: "90s"}}`, "must be below the method timeout"}, + {"service.tools on enumerated", ` cli: {args: ["x"], service: {tools: ["a"]}}`, "tools only applies to a passthrough route"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cfg, err := Parse([]byte(base + tc.method + "\n")) + if err != nil { + t.Fatalf("parse: %v", err) + } + cfg.Resolve() + var msgs []string + for _, e := range cfg.Validate() { + msgs = append(msgs, e.Error()) + } + all := strings.Join(msgs, "\n") + if tc.want == "" { + for _, m := range msgs { + if strings.Contains(m, "cli.") { + t.Errorf("unexpected cli error: %s", m) + } + } + return + } + if !strings.Contains(all, tc.want) { + t.Errorf("errors %q do not mention %q", all, tc.want) + } + }) + } +} diff --git a/internal/scaffold/zz_cli_service_lifetime_test.go b/internal/scaffold/zz_cli_service_lifetime_test.go new file mode 100644 index 0000000..9fe5b19 --- /dev/null +++ b/internal/scaffold/zz_cli_service_lifetime_test.go @@ -0,0 +1,364 @@ +//go:build !windows + +package scaffold + +import ( + "bufio" + "encoding/json" + "fmt" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + "github.com/pilot-protocol/app-store/pkg/ipc" +) + +// TestCLIServiceLifetime: a server started by a cli.service method must not +// outlive the adapter, however the adapter goes away. Found with io.pilot.redis +// 8.6.2: redis.start ran `redis-server --daemonize yes`, whose server setsid()s +// out of the adapter's process group and survived the adapter's SIGTERM, the +// supervisor's SIGKILL of the adapter's pid, and the supervisor's own death, on +// Linux and macOS. A passthrough redis.exec could do the same, or run any host +// binary through a relative args[0] ("../../../../bin/sh"). +// +// - the server stays a foreground child in the adapter's group, with +// force_args appended last (so --daemonize no always wins); +// - SIGTERM: StopServices stops it, and the group is left empty; +// - SIGKILL of the adapter: the child guard, held while the server runs, +// takes the group down (the only mechanism on macOS; Linux adds Pdeathsig); +// - supervisor death: the adapter shuts down (macOS: parent watch; Linux: the +// supervisor's Pdeathsig kills the adapter, the server's Pdeathsig and the +// guard do the rest); +// - passthrough: args[0] must be in cli.tools; a one-shot service tool +// (--version) replies like a command; a busy ready_tcp fails fast; a server +// that never gets ready is stopped. +func TestCLIServiceLifetime(t *testing.T) { + if testing.Short() { + t.Skip("builds and runs a real adapter binary; skipped under -short") + } + if _, err := exec.LookPath("go"); err != nil { + t.Skip("go toolchain not available") + } + root := t.TempDir() + tool := filepath.Join(root, "svctool") + // `serve ...` records "pid argv" and runs until signalled; a + // SIGTERM (a clean stop, which lets a real server save its data) leaves + // .term behind, a SIGKILL leaves nothing. `quick` is a one-shot + // that exits before it could count as ready. + script := "#!/bin/sh\n" + + "case \"$1\" in\n" + + " serve) echo \"$$ $*\" > \"$2\"; trap 'echo term > \"$2.term\"; kill $! 2>/dev/null; exit 0' TERM; sleep 300 & wait;;\n" + + " quick) echo quick-out; exit 3;;\n" + + " *) echo '{}';;\n" + + "esac\n" + if err := os.WriteFile(tool, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + cfg := parseSpec(t, ` +id: io.pilot.svctool +app_version: 0.1.0 +description: "Fronts svctool." +namespace: svctool +backend: + type: cli + command: ["`+tool+`"] +methods: + - name: svctool.start + summary: "Start a server." + cli: + args: ["serve", "${pidfile}"] + service: {ready_after: "300ms", ready_timeout: "10s", force_args: ["--daemonize", "no"]} + - name: svctool.listen + summary: "Start a server that must answer on a port." + cli: + args: ["serve", "${pidfile}"] + service: {ready_tcp: "127.0.0.1:${port}", ready_timeout: "2s"} + - name: svctool.exec + summary: "Passthrough." + cli: + passthrough: true + tools: ["serve", "quick"] + service: {tools: ["serve"], ready_after: "300ms", force_args: ["--daemonize", "no"]} +`) + proj := filepath.Join(root, "proj") + if _, err := Generate(cfg, proj); err != nil { + t.Fatalf("generate: %v", err) + } + if sum, err := os.ReadFile(filepath.Join("..", "..", "go.sum")); err == nil { + _ = os.WriteFile(filepath.Join(proj, "go.sum"), sum, 0o644) + } + bin := filepath.Join(root, "adapter") + build := exec.Command("go", "build", "-o", bin, "./cmd/"+cfg.BinaryName) + build.Dir = proj + build.Env = append(os.Environ(), "GOFLAGS=-mod=mod") + if out, err := build.CombinedOutput(); err != nil { + t.Fatalf("build adapter: %v\n%s", err, out) + } + manifest := filepath.Join(proj, "manifest.json") + + shortDir := func(t *testing.T) string { + t.Helper() + dir, err := os.MkdirTemp("", "svc") // short: sun_path is ~104 bytes on darwin + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(dir) }) + return dir + } + waitFor := func(t *testing.T, cond func() bool, d time.Duration, what string) { + t.Helper() + for deadline := time.Now().Add(d); ; time.Sleep(20 * time.Millisecond) { + if cond() { + return + } + if time.Now().After(deadline) { + t.Fatalf("timed out after %s waiting for %s", d, what) + } + } + } + waitSock := func(t *testing.T, sock string) { + t.Helper() + waitFor(t, func() bool { _, err := os.Stat(sock); return err == nil }, 10*time.Second, "adapter socket") + } + // spawn starts the adapter as the supervisor does: leader of its own group. + spawn := func(t *testing.T, sock string) *exec.Cmd { + t.Helper() + a := exec.Command(bin, "--socket", sock, "--manifest", manifest) + a.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + a.Stderr = os.Stderr + if err := a.Start(); err != nil { + t.Fatalf("start adapter: %v", err) + } + pid := a.Process.Pid + t.Cleanup(func() { _ = syscall.Kill(-pid, syscall.SIGKILL); _, _ = a.Process.Wait() }) + waitSock(t, sock) + return a + } + call := func(sock, method string, args any) (map[string]any, error) { + conn, err := net.DialTimeout("unix", sock, 3*time.Second) + if err != nil { + return nil, err + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(20 * time.Second)) + var out json.RawMessage + if err := ipc.Call(conn, method, args, &out); err != nil { + return nil, err + } + var m map[string]any + _ = json.Unmarshal(out, &m) + return m, nil + } + readPid := func(t *testing.T, pidFile string) (int, string) { + t.Helper() + var pid int + var line string + waitFor(t, func() bool { + b, err := os.ReadFile(pidFile) + if err != nil { + return false + } + line = strings.TrimSpace(string(b)) + f := strings.Fields(line) + if len(f) == 0 { + return false + } + pid, err = strconv.Atoi(f[0]) + return err == nil && pid > 0 + }, 10*time.Second, "server pid file") + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) + return pid, line + } + startArgs := func(method, pidFile string) any { + if method == "svctool.exec" { + return map[string]any{"args": []string{"serve", pidFile}} + } + return map[string]string{"pidfile": pidFile} + } + + stops := []struct { + name string + stop func(t *testing.T, a *exec.Cmd) + }{ + {"sigterm", func(t *testing.T, a *exec.Cmd) { + _ = a.Process.Signal(syscall.SIGTERM) + st, _ := a.Process.Wait() + if st == nil || !st.Success() { + t.Errorf("adapter exit after SIGTERM = %v, want 0", st) + } + }}, + // The app-store supervisor's stop path through v1.0.3: SIGKILL to the + // adapter's pid only. + {"sigkill", func(t *testing.T, a *exec.Cmd) { + _ = a.Process.Kill() + _, _ = a.Process.Wait() + }}, + } + for _, tc := range stops { + for _, method := range []string{"svctool.start", "svctool.exec"} { + t.Run(tc.name+"/"+method, func(t *testing.T) { + dir := shortDir(t) + sock := filepath.Join(dir, "app.sock") + pidFile := filepath.Join(dir, "srv.pid") + a := spawn(t, sock) + apid := a.Process.Pid + res, err := call(sock, method, startArgs(method, pidFile)) + if err != nil { + t.Fatalf("%s: %v", method, err) + } + if res["exit"] != float64(0) || res["pid"] == nil { + t.Fatalf("%s reply = %v, want exit 0 with a pid", method, res) + } + srv, argv := readPid(t, pidFile) + if !strings.HasSuffix(argv, "--daemonize no") { + t.Errorf("server argv %q: force_args must come last", argv) + } + if pg, err := syscall.Getpgid(srv); err != nil || pg != apid { + t.Errorf("server pgid = %d (err %v), want the adapter's group %d", pg, err, apid) + } + // A second call while the first server runs: the adapter keeps + // serving, and the guard it holds does not end the call. + if res, err := call(sock, "svctool.exec", map[string]any{"args": []string{"quick"}}); err != nil || res["exit"] != float64(3) { + t.Errorf("quick while a server runs = %v, %v", res, err) + } + // Past the guard's idle retirement (guardIdle, 1s) after that + // call: from here on only the running server keeps a guard. + time.Sleep(1500 * time.Millisecond) + if n := len(groupMembers(apid)); n < 3 { + t.Errorf("group %d has %d live members, want adapter + server + child guard", apid, n) + } + tc.stop(t, a) + waitFor(t, func() bool { return !procAlive(srv) }, 10*time.Second, "server to die after adapter "+tc.name) + if left := groupLeft(apid, 3*time.Second); len(left) > 0 { + t.Errorf("adapter's process group %d not empty after %s: %v", apid, tc.name, left) + } + // A clean stop of the adapter stops its server cleanly too + // (SIGTERM from StopServices), not by the guard's SIGKILL. + if _, err := os.Stat(pidFile + ".term"); tc.name == "sigterm" && err != nil { + t.Errorf("server was not stopped with SIGTERM on the adapter's clean exit (no %s)", filepath.Base(pidFile)+".term") + } + }) + } + } + + t.Run("supervisor-death", func(t *testing.T) { + dir := shortDir(t) + sock := filepath.Join(dir, "app.sock") + pidFile := filepath.Join(dir, "srv.pid") + argv, _ := json.Marshal([]string{bin, "--socket", sock, "--manifest", manifest}) + sup := exec.Command(os.Args[0], "-test.run=^TestHelperMiniSupervisor$", "-test.v=false") + sup.Env = append(os.Environ(), miniSupEnv+"="+string(argv)) + sup.Stderr = os.Stderr + out, err := sup.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := sup.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = sup.Process.Kill(); _, _ = sup.Process.Wait() }) + line, err := bufio.NewReader(out).ReadString('\n') + if err != nil { + t.Fatalf("read adapter pid from mini supervisor: %v", err) + } + apid, err := strconv.Atoi(strings.TrimSpace(line)) + if err != nil { + t.Fatalf("adapter pid %q: %v", line, err) + } + t.Cleanup(func() { _ = syscall.Kill(-apid, syscall.SIGKILL) }) + waitSock(t, sock) + if res, err := call(sock, "svctool.start", map[string]string{"pidfile": pidFile}); err != nil || res["exit"] != float64(0) { + t.Fatalf("svctool.start = %v, %v", res, err) + } + srv, _ := readPid(t, pidFile) + _ = sup.Process.Kill() // the daemon crashes + _, _ = sup.Process.Wait() + waitFor(t, func() bool { return !procAlive(apid) }, 5*time.Second, "adapter to exit after its supervisor died") + waitFor(t, func() bool { return !procAlive(srv) }, 10*time.Second, "server to stop with the adapter") + if left := groupLeft(apid, 3*time.Second); len(left) > 0 { + t.Errorf("adapter's process group %d not empty after supervisor death: %v", apid, left) + } + }) + + t.Run("replies", func(t *testing.T) { + dir := shortDir(t) + sock := filepath.Join(dir, "app.sock") + spawn(t, sock) + // A one-shot passthrough service tool returns its output like a command. + res, err := call(sock, "svctool.exec", map[string]any{"args": []string{"quick"}}) + if err != nil || res["exit"] != float64(3) || res["stdout"] != "quick-out" { + t.Errorf("quick = %v, %v; want exit 3 stdout quick-out", res, err) + } + // args[0] outside cli.tools (a path, a shell, nothing) is refused. + for _, bad := range [][]string{{"../../../../bin/sh", "-c", "true"}, {"sh"}, {"./serve"}, {}} { + if _, err := call(sock, "svctool.exec", map[string]any{"args": bad}); err == nil || !strings.Contains(err.Error(), "args[0] must be one of") { + t.Errorf("args %q: err = %v, want the tools allowlist refusal", bad, err) + } + } + // ready_tcp: something already listening on the port fails fast and + // starts nothing. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + port := strconv.Itoa(ln.Addr().(*net.TCPAddr).Port) + pf := filepath.Join(dir, "busy.pid") + if _, err := call(sock, "svctool.listen", map[string]string{"pidfile": pf, "port": port}); err == nil || !strings.Contains(err.Error(), "already accepting connections") { + t.Errorf("listen on a busy port: err = %v, want already-accepting refusal", err) + } + if _, err := os.Stat(pf); err == nil { + t.Errorf("server was started although the port was busy") + } + // ready_tcp never satisfied: the server is stopped and the call fails. + ln2, _ := net.Listen("tcp", "127.0.0.1:0") + free := strconv.Itoa(ln2.Addr().(*net.TCPAddr).Port) + ln2.Close() + pf2 := filepath.Join(dir, "never.pid") + if _, err := call(sock, "svctool.listen", map[string]string{"pidfile": pf2, "port": free}); err == nil || !strings.Contains(err.Error(), "not ready within") { + t.Errorf("never-ready server: err = %v, want not-ready failure", err) + } + srv, _ := readPid(t, pf2) + waitFor(t, func() bool { return !procAlive(srv) }, 12*time.Second, "never-ready server to be stopped") + }) + + // Several services at once all stop on SIGTERM, within the grace. + t.Run("sigterm/many", func(t *testing.T) { + dir := shortDir(t) + sock := filepath.Join(dir, "app.sock") + a := spawn(t, sock) + apid := a.Process.Pid + var srvs []int + for i := 0; i < 3; i++ { + pf := filepath.Join(dir, fmt.Sprintf("m%d.pid", i)) + if _, err := call(sock, "svctool.start", map[string]string{"pidfile": pf}); err != nil { + t.Fatalf("start %d: %v", i, err) + } + pid, _ := readPid(t, pf) + srvs = append(srvs, pid) + } + t0 := time.Now() + _ = a.Process.Signal(syscall.SIGTERM) + _, _ = a.Process.Wait() + if d := time.Since(t0); d > 5*time.Second { + t.Errorf("SIGTERM with 3 services took %s", d) + } + for i, p := range srvs { + if procAlive(p) { + t.Errorf("server %d alive after the adapter's SIGTERM exit", p) + } + if _, err := os.Stat(filepath.Join(dir, fmt.Sprintf("m%d.pid.term", i))); err != nil { + t.Errorf("server %d was not stopped with SIGTERM", p) + } + } + if left := groupMembers(apid); len(left) > 0 { + t.Errorf("process group %d not empty after SIGTERM: %v", apid, left) + } + }) +} diff --git a/submissions/io.pilot.redis/submission.json b/submissions/io.pilot.redis/submission.json index 90d5c3d..e9dcb57 100644 --- a/submissions/io.pilot.redis/submission.json +++ b/submissions/io.pilot.redis/submission.json @@ -1,6 +1,6 @@ { "id": "io.pilot.redis", - "version": "8.6.2", + "version": "8.6.3", "description": "Redis 8.6.2 as a native CLI for agents: stand up a local in-memory data store (redis.start) and talk to it with redis-cli — SET/GET, PING, INFO, DBSIZE, and ANY Redis command (lists, hashes, sets, sorted sets, streams, pub/sub, transactions) via a verbatim-argv passthrough. Server lifecycle (start/stop), health checks, and the full redis-cli/redis-server toolchain (redis-benchmark, redis-check-rdb, ...). No server to provision — runs locally on this host.", "email": "apps@pilotprotocol.network", "backend": { @@ -17,7 +17,7 @@ "methods": [ { "name": "redis.start", - "description": "Start a local Redis server, daemonized, listening on 127.0.0.1:`port`. Writes its pidfile, logfile, and RDB snapshot under `dir`; returns once the server has forked into the background. After this, use redis.ping/redis.set/redis.get/redis.exec against the same `port`. This is `redis-server --port --dir --daemonize yes --pidfile /redis-.pid --logfile /redis-.log`.", + "description": "Start a local Redis server listening on 127.0.0.1:`port`. Writes its pidfile, logfile, and RDB snapshot under `dir`; returns once the server accepts connections. The server runs under this app (a foreground child, never daemonized) and stops when the app stops, restarts, or is uninstalled, so it cannot outlive the app. Fails if something already listens on 127.0.0.1:`port`. After this, use redis.ping/redis.set/redis.get/redis.exec against the same `port`. This is `redis-server --port --bind 127.0.0.1 --dir --pidfile /redis-.pid --logfile /redis-.log --daemonize no`.", "latency": "med", "params": [ { @@ -38,15 +38,21 @@ "redis-server", "--port", "${port}", + "--bind", + "127.0.0.1", "--dir", "${dir}", - "--daemonize", - "yes", "--pidfile", "${dir}/redis-${port}.pid", "--logfile", "${dir}/redis-${port}.log" - ] + ], + "service": { + "ready_tcp": "127.0.0.1:${port}", + "ready_timeout": "30s", + "log_file": "${dir}/redis-${port}.log", + "force_args": ["--daemonize", "no"] + } } }, { @@ -199,7 +205,7 @@ }, { "name": "redis.exec", - "description": "Run any bundled Redis tool with a verbatim argv — the full surface beyond the curated methods. Payload is {\"args\":[, ...]} where the first element is the tool (redis-cli, redis-server, redis-benchmark, redis-check-rdb, redis-check-aof, redis-sentinel) and the rest are its args; optional {\"stdin\":\"...\"} is piped to it. This is how you run ANY Redis command: {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"LPUSH\",\"mylist\",\"a\",\"b\",\"c\"]}, {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"-n\",\"1\",\"HSET\",\"h\",\"f\",\"v\"]}, a pipelined script via stdin {\"args\":[\"redis-cli\",\"-p\",\"6399\"],\"stdin\":\"SET a 1\\nINCR a\\nGET a\"}, or a benchmark {\"args\":[\"redis-benchmark\",\"-p\",\"6399\",\"-n\",\"1000\",\"-q\"]}. The REDISCLI_AUTH env var is passed through for password auth.", + "description": "Run any bundled Redis tool with a verbatim argv — the full surface beyond the curated methods. Payload is {\"args\":[, ...]} where the first element is the tool (redis-cli, redis-server, redis-benchmark, redis-check-rdb, redis-check-aof, redis-sentinel) and the rest are its args; optional {\"stdin\":\"...\"} is piped to it. This is how you run ANY Redis command: {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"LPUSH\",\"mylist\",\"a\",\"b\",\"c\"]}, {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"-n\",\"1\",\"HSET\",\"h\",\"f\",\"v\"]}, a pipelined script via stdin {\"args\":[\"redis-cli\",\"-p\",\"6399\"],\"stdin\":\"SET a 1\\nINCR a\\nGET a\"}, or a benchmark {\"args\":[\"redis-benchmark\",\"-p\",\"6399\",\"-n\",\"1000\",\"-q\"]}. The REDISCLI_AUTH env var is passed through for password auth. args[0] must be one of the six bundled tools. A redis-server or redis-sentinel started here runs like redis.start: under this app, kept in the foreground (--daemonize no is appended), returned once it has stayed up for 1s (a --help/--version/config-error run just returns its output), and stopped when the app stops.", "latency": "med", "params": [ { @@ -216,7 +222,14 @@ } ], "cli": { - "passthrough": true + "passthrough": true, + "tools": ["redis-server", "redis-cli", "redis-benchmark", "redis-check-rdb", "redis-check-aof", "redis-sentinel"], + "service": { + "tools": ["redis-server", "redis-sentinel"], + "force_args": ["--daemonize", "no"], + "ready_after": "1s", + "ready_timeout": "30s" + } } }, { @@ -245,7 +258,7 @@ "listing": { "display_name": "Redis", "tagline": "Run Redis from an agent — start a local in-memory store and run any command with redis-cli", - "app_description": "# Redis (server + redis-cli) — native CLI for agents\n\nThis app installs the official **Redis 8.6.2** server and client on the host and fronts them as\ntyped methods. The bundle is a relocatable build of Redis 8.6.2 (from conda-forge, AGPL-3.0)\ncarrying `redis-server`, `redis-cli`, `redis-benchmark`, `redis-check-rdb`, `redis-check-aof`, and\n`redis-sentinel`; every binary is sha-pinned and staged at install, and a tiny `redis` dispatcher routes\neach method to the right tool. Binaries are fetched from the Pilot artifact registry on **macOS and Linux**\n(arm64 + amd64).\n\nRedis is an **in-memory data-structure store** — strings, hashes, lists, sets, sorted sets, streams, plus\npub/sub and transactions. There is no cluster to provision: an agent starts a throwaway local server and\nuses it as a cache, a fast key-value DB, or a coordination/queue primitive.\n\n## Run a Redis locally — the usual flow\n\n1. **Start:** `redis.start` `{ \"port\": \"6399\", \"dir\": \"/tmp\" }` — boots a daemonized server on\n `127.0.0.1:6399`, with pidfile/logfile/RDB under `dir`.\n2. **Health:** `redis.ping` `{ \"port\": \"6399\" }` → `PONG`.\n3. **Use it:** `redis.set` / `redis.get`, `redis.info`, `redis.dbsize`, or **any** command via\n `redis.exec` `{ \"args\": [\"redis-cli\",\"-p\",\"6399\",\"ZADD\",\"board\",\"100\",\"alice\"] }`.\n4. **Stop:** `redis.stop` `{ \"port\": \"6399\" }`.\n\n## Methods\n\n- `redis.start` / `redis.stop` — local server lifecycle (daemonized; per-port pidfile/logfile).\n- `redis.ping` — liveness (PONG). `redis.info` — full server INFO. `redis.dbsize` — key count.\n- `redis.set` / `redis.get` — string get/set convenience.\n- `redis.exec` — run any tool with a verbatim argv (+ optional stdin) — every Redis command, pipelined\n batches, benchmarks, RDB/AOF checks, sentinel.\n- `redis.cli_help` — the complete `redis-cli --help`. `redis.version` — the delivered version.\n `redis.help` — the self-describing method list.\n\n## Configuration\n\n- **`port`** — TCP port for the local server (convention `6399`). Bound to `127.0.0.1`.\n- **`dir`** — an existing writable directory for the pidfile, logfile, and RDB snapshot; pass `/tmp` for a\n throwaway server, or a path you control for persistence (the RDB lives at `/dump.rdb`).\n- **Persistence** — `redis.stop` does `SHUTDOWN NOSAVE`; run `redis.exec` with `SAVE`/`BGSAVE` first if you\n need the dataset on disk. Any redis.conf directive is reachable as a `--flag` via\n `redis.exec` (`redis-server --port ... --maxmemory 256mb --maxmemory-policy allkeys-lru ...`).\n- **Auth** — local servers are open by default; set `--requirepass` (via `redis.exec` on start) and pass the\n password through the `REDISCLI_AUTH` env var (forwarded to the child) or `-a` on `redis.exec`.\n\n## Good to know\n\n- Output returns verbatim where it is already clean; on a non-zero exit the reply is `{stdout, stderr, exit}`.\n- Free and open source under the **AGPL-3.0** license (Redis 8.x). Repackaged unmodified from conda-forge.\n\n## redis-cli / redis-server help\n```\nRedis CLI help — redis-cli and redis-server\n===========================================\n\nRedis is an in-memory data-structure store (cache, database, message broker).\nThis app fronts the Redis server + redis-cli as agent methods: start a local\nserver, then SET/GET/PING/INFO or run any command via redis.exec.\n\n------------------------------------------------------------------------------\nredis-cli --help\n------------------------------------------------------------------------------\nredis-cli 8.6.2\n\nUsage: redis-cli [OPTIONS] [cmd [arg [arg ...]]]\n -h Server hostname (default: 127.0.0.1).\n -p Server port (default: 6379).\n -t Server connection timeout in seconds (decimals allowed).\n Default timeout is 0, meaning no limit, depending on the OS.\n -s Server socket (overrides hostname and port).\n -a Password to use when connecting to the server.\n You can also use the REDISCLI_AUTH environment\n variable to pass this password more safely\n (if both are used, this argument takes precedence).\n --user Used to send ACL style 'AUTH username pass'. Needs -a.\n --pass Alias of -a for consistency with the new --user option.\n --askpass Force user to input password with mask from STDIN.\n If this argument is used, '-a' and REDISCLI_AUTH\n environment variable will be ignored.\n -u Server URI on format redis://user:password@host:port/dbnum\n User, password and dbnum are optional. For authentication\n without a username, use username 'default'. For TLS, use\n the scheme 'rediss'.\n -r Execute specified command N times.\n -i When -r is used, waits seconds per command.\n It is possible to specify sub-second times like -i 0.1.\n This interval is also used in --scan and --stat per cycle.\n and in --bigkeys, --memkeys, --keystats, and --hotkeys per 100 cycles.\n -n Database number.\n --name Set the client name.\n -2 Start session in RESP2 protocol mode.\n -3 Start session in RESP3 protocol mode.\n -x Read last argument from STDIN (see example below).\n -X Read argument from STDIN (see example below).\n -d Delimiter between response bulks for raw formatting (default: \\n).\n -D Delimiter between responses for raw formatting (default: \\n).\n -c Enable cluster mode (follow -ASK and -MOVED redirections).\n -e Return exit error code when command execution fails.\n -4 Prefer IPv4 over IPv6 on DNS lookup.\n -6 Prefer IPv6 over IPv4 on DNS lookup.\n --tls Establish a secure TLS connection.\n --sni Server name indication for TLS.\n --cacert CA Certificate file to verify with.\n --cacertdir Directory where trusted CA certificates are stored.\n If neither cacert nor cacertdir are specified, the default\n system-wide trusted root certs configuration will apply.\n --insecure Allow insecure TLS connection by skipping cert validation.\n --cert Client certificate to authenticate with.\n --key Private key file to authenticate with.\n --tls-ciphers Sets the list of preferred ciphers (TLSv1.2 and below)\n in order of preference from highest to lowest separated by colon (\":\").\n See the ciphers(1ssl) manpage for more information about the syntax of this string.\n --tls-ciphersuites Sets the list of preferred ciphersuites (TLSv1.3)\n in order of preference from highest to lowest separated by colon (\":\").\n See the ciphers(1ssl) manpage for more information about the syntax of this string,\n and specifically for TLSv1.3 ciphersuites.\n --raw Use raw formatting for replies (default when STDOUT is\n not a tty).\n --no-raw Force formatted output even when STDOUT is not a tty.\n --quoted-input Force input to be handled as quoted strings.\n --csv Output in CSV format.\n --json Output in JSON format (default RESP3, use -2 if you want to use with RESP2).\n --quoted-json Same as --json, but produce ASCII-safe quoted strings, not Unicode.\n --show-pushes Whether to print RESP3 PUSH messages. Enabled by default when\n STDOUT is a tty but can be overridden with --show-pushes no.\n --stat Print rolling stats about server: mem, clients, ...\n --latency Enter a special mode continuously sampling latency.\n If you use this mode in an interactive session it runs\n forever displaying real-time stats. Otherwise if --raw or\n --csv is specified, or if you redirect the output to a non\n TTY, it samples the latency for 1 second (you can use\n -i to change the interval), then produces a single output\n and exits.\n --latency-history Like --latency but tracking latency changes over time.\n Default time interval is 15 sec. Change it using -i.\n --latency-dist Shows latency as a spectrum, requires xterm 256 colors.\n Default time interval is 1 sec. Change it using -i.\n --vset-recall Enable VSIM recall test mode for the specified key\n (that must be a vector set). Random vectors are created\n mixing components from other elements. A VSIM is then\n executed and checked against ground truth.\n --vset-recall-count How many top elements to fetch per query.\n --vset-recall-ef HSNW EF (search effort) to use. Default 500.\n --vset-recall-ele Number of elements used to compose query vectors\n Default 1.\n --lru-test Simulate a cache workload with an 80-20 distribution.\n --replica Simulate a replica showing commands received from the master.\n --rdb Transfer an RDB dump from remote server to local file.\n Use filename of \"-\" to write to stdout.\n --functions-rdb Like --rdb but only get the functions (not the keys)\n when getting the RDB dump file.\n --pipe Transfer raw Redis protocol from stdin to server.\n --pipe-timeout In --pipe mode, abort with error if after sending all data.\n no reply is received within seconds.\n Default timeout: 30. Use 0 to wait forever.\n --bigkeys Sample Redis keys looking for keys with many elements (complexity).\n --memkeys Sample Redis keys looking for keys consuming a lot of memory.\n --memkeys-samples Sample Redis keys looking for keys consuming a lot of memory.\n And define number of key elements to sample\n --keystats Sample Redis keys looking for keys memory size and length (combine bigkeys and memkeys).\n --keystats-samples Sample Redis keys looking for keys memory size and length.\n And define number of key elements to sample (only for memory usage).\n --cursor Start the scan at the cursor (usually after a Ctrl-C).\n Optionally used with --keystats and --keystats-samples.\n --top To display top key sizes (default: 10).\n Optionally used with --keystats and --keystats-samples.\n --hotkeys Sample Redis keys looking for hot keys.\n only works when maxmemory-policy is *lfu.\n --scan List all keys using the SCAN command.\n --pattern Keys pattern when using the --scan, --bigkeys, --memkeys,\n --keystats or --hotkeys options (default: *).\n --count Count option when using the --scan, --bigkeys, --memkeys,\n --keystats or --hotkeys (default: 10).\n --quoted-pattern Same as --pattern, but the specified string can be\n quoted, in order to pass an otherwise non binary-safe string.\n --intrinsic-latency Run a test to measure intrinsic system latency.\n The test will run for the specified amount of seconds.\n --eval Send an EVAL command using the Lua script at .\n --ldb Used with --eval enable the Redis Lua debugger.\n --ldb-sync-mode Like --ldb but uses the synchronous Lua debugger, in\n this mode the server is blocked and script changes are\n not rolled back from the server memory.\n --cluster [args...] [opts...]\n Cluster Manager command and arguments (see below).\n --verbose Verbose mode.\n --no-auth-warning Don't show warning message when using password on command\n line interface.\n --help Output this help and exit.\n --version Output version and exit.\n\nCluster Manager Commands:\n Use --cluster help to list all available cluster manager commands.\n\nExamples:\n redis-cli -u redis://default:PASSWORD@localhost:6379/0\n cat /etc/passwd | redis-cli -x set mypasswd\n redis-cli -D \"\" --raw dump key > key.dump && redis-cli -X dump_tag restore key2 0 dump_tag replace < key.dump\n redis-cli -r 100 lpush mylist x\n redis-cli -r 100 -i 1 info | grep used_memory_human:\n redis-cli --quoted-input set '\"null-\\x00-separated\"' value\n redis-cli --eval myscript.lua key1 key2 , arg1 arg2 arg3\n redis-cli --scan --pattern '*:12345*'\n redis-cli --scan --pattern '*:12345*' --count 100\n\n (Note: when using --eval the comma separates KEYS[] from ARGV[] items)\n\nWhen no command is given, redis-cli starts in interactive mode.\nType \"help\" in interactive mode for information on available commands\nand settings.\n\n\n------------------------------------------------------------------------------\nredis-server --help\n------------------------------------------------------------------------------\nUsage: ./redis-server [/path/to/redis.conf] [options] [-]\n ./redis-server - (read config from stdin)\n ./redis-server -v or --version\n ./redis-server -h or --help\n ./redis-server --test-memory \n ./redis-server --check-system\n\nExamples:\n ./redis-server (run the server with default conf)\n echo 'maxmemory 128mb' | ./redis-server -\n ./redis-server /etc/redis/6379.conf\n ./redis-server --port 7777\n ./redis-server --port 7777 --replicaof 127.0.0.1 8888\n ./redis-server /etc/myredis.conf --loglevel verbose -\n ./redis-server /etc/myredis.conf --loglevel verbose\n\nSentinel mode:\n ./redis-server /etc/sentinel.conf --sentinel\n```\n", + "app_description": "# Redis (server + redis-cli) — native CLI for agents\n\nThis app installs the official **Redis 8.6.2** server and client on the host and fronts them as\ntyped methods. The bundle is a relocatable build of Redis 8.6.2 (from conda-forge, AGPL-3.0)\ncarrying `redis-server`, `redis-cli`, `redis-benchmark`, `redis-check-rdb`, `redis-check-aof`, and\n`redis-sentinel`; every binary is sha-pinned and staged at install, and a tiny `redis` dispatcher routes\neach method to the right tool. Binaries are fetched from the Pilot artifact registry on **macOS and Linux**\n(arm64 + amd64).\n\nRedis is an **in-memory data-structure store** — strings, hashes, lists, sets, sorted sets, streams, plus\npub/sub and transactions. There is no cluster to provision: an agent starts a throwaway local server and\nuses it as a cache, a fast key-value DB, or a coordination/queue primitive.\n\n## Run a Redis locally — the usual flow\n\n1. **Start:** `redis.start` `{ \"port\": \"6399\", \"dir\": \"/tmp\" }` — boots a server on\n `127.0.0.1:6399`, with pidfile/logfile/RDB under `dir`; it runs under the app and stops when the app stops.\n2. **Health:** `redis.ping` `{ \"port\": \"6399\" }` → `PONG`.\n3. **Use it:** `redis.set` / `redis.get`, `redis.info`, `redis.dbsize`, or **any** command via\n `redis.exec` `{ \"args\": [\"redis-cli\",\"-p\",\"6399\",\"ZADD\",\"board\",\"100\",\"alice\"] }`.\n4. **Stop:** `redis.stop` `{ \"port\": \"6399\" }`.\n\n## Methods\n\n- `redis.start` / `redis.stop` — local server lifecycle (app-managed, never daemonized; per-port pidfile/logfile).\n- `redis.ping` — liveness (PONG). `redis.info` — full server INFO. `redis.dbsize` — key count.\n- `redis.set` / `redis.get` — string get/set convenience.\n- `redis.exec` — run any tool with a verbatim argv (+ optional stdin) — every Redis command, pipelined\n batches, benchmarks, RDB/AOF checks, sentinel.\n- `redis.cli_help` — the complete `redis-cli --help`. `redis.version` — the delivered version.\n `redis.help` — the self-describing method list.\n\n## Configuration\n\n- **`port`** — TCP port for the local server (convention `6399`). Bound to `127.0.0.1`.\n- **`dir`** — an existing writable directory for the pidfile, logfile, and RDB snapshot; pass `/tmp` for a\n throwaway server, or a path you control for persistence (the RDB lives at `/dump.rdb`).\n- **Persistence** — `redis.stop` does `SHUTDOWN NOSAVE`; run `redis.exec` with `SAVE`/`BGSAVE` first if you\n need the dataset on disk. Any redis.conf directive is reachable as a `--flag` via\n `redis.exec` (`redis-server --port ... --maxmemory 256mb --maxmemory-policy allkeys-lru ...`).\n- **Auth** — local servers are open by default; set `--requirepass` (via `redis.exec` on start) and pass the\n password through the `REDISCLI_AUTH` env var (forwarded to the child) or `-a` on `redis.exec`.\n\n## Good to know\n\n- Output returns verbatim where it is already clean; on a non-zero exit the reply is `{stdout, stderr, exit}`.\n- Free and open source under the **AGPL-3.0** license (Redis 8.x). Repackaged unmodified from conda-forge.\n\n## redis-cli / redis-server help\n```\nRedis CLI help — redis-cli and redis-server\n===========================================\n\nRedis is an in-memory data-structure store (cache, database, message broker).\nThis app fronts the Redis server + redis-cli as agent methods: start a local\nserver, then SET/GET/PING/INFO or run any command via redis.exec.\n\n------------------------------------------------------------------------------\nredis-cli --help\n------------------------------------------------------------------------------\nredis-cli 8.6.2\n\nUsage: redis-cli [OPTIONS] [cmd [arg [arg ...]]]\n -h Server hostname (default: 127.0.0.1).\n -p Server port (default: 6379).\n -t Server connection timeout in seconds (decimals allowed).\n Default timeout is 0, meaning no limit, depending on the OS.\n -s Server socket (overrides hostname and port).\n -a Password to use when connecting to the server.\n You can also use the REDISCLI_AUTH environment\n variable to pass this password more safely\n (if both are used, this argument takes precedence).\n --user Used to send ACL style 'AUTH username pass'. Needs -a.\n --pass Alias of -a for consistency with the new --user option.\n --askpass Force user to input password with mask from STDIN.\n If this argument is used, '-a' and REDISCLI_AUTH\n environment variable will be ignored.\n -u Server URI on format redis://user:password@host:port/dbnum\n User, password and dbnum are optional. For authentication\n without a username, use username 'default'. For TLS, use\n the scheme 'rediss'.\n -r Execute specified command N times.\n -i When -r is used, waits seconds per command.\n It is possible to specify sub-second times like -i 0.1.\n This interval is also used in --scan and --stat per cycle.\n and in --bigkeys, --memkeys, --keystats, and --hotkeys per 100 cycles.\n -n Database number.\n --name Set the client name.\n -2 Start session in RESP2 protocol mode.\n -3 Start session in RESP3 protocol mode.\n -x Read last argument from STDIN (see example below).\n -X Read argument from STDIN (see example below).\n -d Delimiter between response bulks for raw formatting (default: \\n).\n -D Delimiter between responses for raw formatting (default: \\n).\n -c Enable cluster mode (follow -ASK and -MOVED redirections).\n -e Return exit error code when command execution fails.\n -4 Prefer IPv4 over IPv6 on DNS lookup.\n -6 Prefer IPv6 over IPv4 on DNS lookup.\n --tls Establish a secure TLS connection.\n --sni Server name indication for TLS.\n --cacert CA Certificate file to verify with.\n --cacertdir Directory where trusted CA certificates are stored.\n If neither cacert nor cacertdir are specified, the default\n system-wide trusted root certs configuration will apply.\n --insecure Allow insecure TLS connection by skipping cert validation.\n --cert Client certificate to authenticate with.\n --key Private key file to authenticate with.\n --tls-ciphers Sets the list of preferred ciphers (TLSv1.2 and below)\n in order of preference from highest to lowest separated by colon (\":\").\n See the ciphers(1ssl) manpage for more information about the syntax of this string.\n --tls-ciphersuites Sets the list of preferred ciphersuites (TLSv1.3)\n in order of preference from highest to lowest separated by colon (\":\").\n See the ciphers(1ssl) manpage for more information about the syntax of this string,\n and specifically for TLSv1.3 ciphersuites.\n --raw Use raw formatting for replies (default when STDOUT is\n not a tty).\n --no-raw Force formatted output even when STDOUT is not a tty.\n --quoted-input Force input to be handled as quoted strings.\n --csv Output in CSV format.\n --json Output in JSON format (default RESP3, use -2 if you want to use with RESP2).\n --quoted-json Same as --json, but produce ASCII-safe quoted strings, not Unicode.\n --show-pushes Whether to print RESP3 PUSH messages. Enabled by default when\n STDOUT is a tty but can be overridden with --show-pushes no.\n --stat Print rolling stats about server: mem, clients, ...\n --latency Enter a special mode continuously sampling latency.\n If you use this mode in an interactive session it runs\n forever displaying real-time stats. Otherwise if --raw or\n --csv is specified, or if you redirect the output to a non\n TTY, it samples the latency for 1 second (you can use\n -i to change the interval), then produces a single output\n and exits.\n --latency-history Like --latency but tracking latency changes over time.\n Default time interval is 15 sec. Change it using -i.\n --latency-dist Shows latency as a spectrum, requires xterm 256 colors.\n Default time interval is 1 sec. Change it using -i.\n --vset-recall Enable VSIM recall test mode for the specified key\n (that must be a vector set). Random vectors are created\n mixing components from other elements. A VSIM is then\n executed and checked against ground truth.\n --vset-recall-count How many top elements to fetch per query.\n --vset-recall-ef HSNW EF (search effort) to use. Default 500.\n --vset-recall-ele Number of elements used to compose query vectors\n Default 1.\n --lru-test Simulate a cache workload with an 80-20 distribution.\n --replica Simulate a replica showing commands received from the master.\n --rdb Transfer an RDB dump from remote server to local file.\n Use filename of \"-\" to write to stdout.\n --functions-rdb Like --rdb but only get the functions (not the keys)\n when getting the RDB dump file.\n --pipe Transfer raw Redis protocol from stdin to server.\n --pipe-timeout In --pipe mode, abort with error if after sending all data.\n no reply is received within seconds.\n Default timeout: 30. Use 0 to wait forever.\n --bigkeys Sample Redis keys looking for keys with many elements (complexity).\n --memkeys Sample Redis keys looking for keys consuming a lot of memory.\n --memkeys-samples Sample Redis keys looking for keys consuming a lot of memory.\n And define number of key elements to sample\n --keystats Sample Redis keys looking for keys memory size and length (combine bigkeys and memkeys).\n --keystats-samples Sample Redis keys looking for keys memory size and length.\n And define number of key elements to sample (only for memory usage).\n --cursor Start the scan at the cursor (usually after a Ctrl-C).\n Optionally used with --keystats and --keystats-samples.\n --top To display top key sizes (default: 10).\n Optionally used with --keystats and --keystats-samples.\n --hotkeys Sample Redis keys looking for hot keys.\n only works when maxmemory-policy is *lfu.\n --scan List all keys using the SCAN command.\n --pattern Keys pattern when using the --scan, --bigkeys, --memkeys,\n --keystats or --hotkeys options (default: *).\n --count Count option when using the --scan, --bigkeys, --memkeys,\n --keystats or --hotkeys (default: 10).\n --quoted-pattern Same as --pattern, but the specified string can be\n quoted, in order to pass an otherwise non binary-safe string.\n --intrinsic-latency Run a test to measure intrinsic system latency.\n The test will run for the specified amount of seconds.\n --eval Send an EVAL command using the Lua script at .\n --ldb Used with --eval enable the Redis Lua debugger.\n --ldb-sync-mode Like --ldb but uses the synchronous Lua debugger, in\n this mode the server is blocked and script changes are\n not rolled back from the server memory.\n --cluster [args...] [opts...]\n Cluster Manager command and arguments (see below).\n --verbose Verbose mode.\n --no-auth-warning Don't show warning message when using password on command\n line interface.\n --help Output this help and exit.\n --version Output version and exit.\n\nCluster Manager Commands:\n Use --cluster help to list all available cluster manager commands.\n\nExamples:\n redis-cli -u redis://default:PASSWORD@localhost:6379/0\n cat /etc/passwd | redis-cli -x set mypasswd\n redis-cli -D \"\" --raw dump key > key.dump && redis-cli -X dump_tag restore key2 0 dump_tag replace < key.dump\n redis-cli -r 100 lpush mylist x\n redis-cli -r 100 -i 1 info | grep used_memory_human:\n redis-cli --quoted-input set '\"null-\\x00-separated\"' value\n redis-cli --eval myscript.lua key1 key2 , arg1 arg2 arg3\n redis-cli --scan --pattern '*:12345*'\n redis-cli --scan --pattern '*:12345*' --count 100\n\n (Note: when using --eval the comma separates KEYS[] from ARGV[] items)\n\nWhen no command is given, redis-cli starts in interactive mode.\nType \"help\" in interactive mode for information on available commands\nand settings.\n\n\n------------------------------------------------------------------------------\nredis-server --help\n------------------------------------------------------------------------------\nUsage: ./redis-server [/path/to/redis.conf] [options] [-]\n ./redis-server - (read config from stdin)\n ./redis-server -v or --version\n ./redis-server -h or --help\n ./redis-server --test-memory \n ./redis-server --check-system\n\nExamples:\n ./redis-server (run the server with default conf)\n echo 'maxmemory 128mb' | ./redis-server -\n ./redis-server /etc/redis/6379.conf\n ./redis-server --port 7777\n ./redis-server --port 7777 --replicaof 127.0.0.1 8888\n ./redis-server /etc/myredis.conf --loglevel verbose -\n ./redis-server /etc/myredis.conf --loglevel verbose\n\nSentinel mode:\n ./redis-server /etc/sentinel.conf --sentinel\n```\n", "license": "AGPL-3.0", "homepage": "https://redis.io", "source_url": "https://github.com/redis/redis", @@ -332,7 +345,7 @@ "title": "Start a local server", "goal": "Bring up Redis before any data ops", "command": "pilotctl appstore call io.pilot.redis redis.start '{\"port\":\"6399\",\"dir\":\"/tmp\"}'", - "expect": "server daemonized on 127.0.0.1:6399; pidfile/logfile/RDB written under /tmp" + "expect": "server started on 127.0.0.1:6399 (runs under the app); pidfile/logfile/RDB written under /tmp" }, { "title": "Set a key", @@ -381,7 +394,7 @@ "then": [ { "cmd": "pilotctl appstore call io.pilot.redis redis.start '{\"port\":\"6399\",\"dir\":\"/tmp\"}'", - "why": "start a daemonized server on 127.0.0.1:6399; dir must be an existing writable directory for its pidfile/log/RDB", + "why": "start a server on 127.0.0.1:6399 (it runs under the app); dir must be an existing writable directory for its pidfile/log/RDB", "kind": "gateway" } ] From 6400bfcc6d2469faef9395c279bafbcef8453ce8 Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Thu, 24 Sep 2026 13:39:41 +0300 Subject: [PATCH 2/2] fix(scaffold): a passthrough service is ready when its --port accepts, not after a fixed wait A passthrough service counted as ready once it had stayed up for ready_after (1s for redis.exec). The harness showed that a freshly staged binary's first launch takes longer than that to listen. `redis.start`, which probes its ready_tcp, took this long to become ready in the lifecycle phase: - darwin/arm64: 1.5-1.9s (a relaunch of the same files: 53ms) - darwin/amd64 under Rosetta: 3.2-4.5s (relaunch: 112ms) So `redis.exec ["redis-server","--port",P,...]` reported "started" and the next ping got "Could not connect to Redis at 127.0.0.1:P: Connection refused" (darwin/arm64 and darwin/amd64, 2 of 2 runs). cli.service.ready_port_flag (passthrough only) names the argv flag that carries the server's port. When a call passes it (`--port N` or `--port=N`, the last one wins), the server is ready once 127.0.0.1:N accepts, and the call fails fast if something already accepts there. Without the flag, readiness is still ready_after. redis.exec sets ready_port_flag to "--port". Test: TestCLIServiceLifetime/passthrough-ready-port uses a server that listens only after 1.5s, in both forms, and checks that a busy port is refused. With the port probe disabled it fails ("returned after 309ms, before the server listened"; "reported started but 127.0.0.1:53188 refuses"). Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/CLI-ADAPTER.md | 14 ++- internal/publish/submission.go | 26 ++--- internal/scaffold/config.go | 11 ++ internal/scaffold/templates/main.go.tmpl | 4 +- internal/scaffold/templates/service.go.tmpl | 36 ++++++- .../scaffold/zz_cli_service_config_test.go | 3 + .../scaffold/zz_cli_service_lifetime_test.go | 102 +++++++++++++++++- submissions/io.pilot.redis/submission.json | 3 +- 8 files changed, 175 insertions(+), 24 deletions(-) diff --git a/docs/CLI-ADAPTER.md b/docs/CLI-ADAPTER.md index 579bb00..67eef85 100644 --- a/docs/CLI-ADAPTER.md +++ b/docs/CLI-ADAPTER.md @@ -75,14 +75,22 @@ starts, and a process that exits before it is ready (`--version`, a config error) returns its output like a plain command. On a passthrough route, `service.tools` names the tools that start a server -(needs `cli.tools`); readiness is `ready_after` (default 1 s) of staying up, -since there are no `${field}` params: +(needs `cli.tools`). There are no `${field}` params, so readiness comes from +the argv: with `ready_port_flag`, a call that passes that flag (`--port N` or +`--port=N`) is ready once `127.0.0.1:N` accepts (and fails fast if something +already does); otherwise it is ready after staying up for `ready_after` +(default 1 s). Prefer the port: a freshly staged binary's first launch can +take seconds before it listens (macOS assesses it on first run; Rosetta +translates an amd64 binary), well past `ready_after`. ```yaml cli: passthrough: true tools: [redis-server, redis-cli, redis-sentinel] - service: {tools: [redis-server, redis-sentinel], force_args: [--daemonize, "no"]} + service: + tools: [redis-server, redis-sentinel] + ready_port_flag: --port + force_args: [--daemonize, "no"] ``` ## Hardening built into the runner diff --git a/internal/publish/submission.go b/internal/publish/submission.go index fc125db..fa636cf 100644 --- a/internal/publish/submission.go +++ b/internal/publish/submission.go @@ -240,12 +240,13 @@ type SubCLIRoute struct { // SubCLIService mirrors scaffold.CLIService. type SubCLIService struct { - ReadyTCP string `json:"ready_tcp,omitempty"` - ReadyAfter string `json:"ready_after,omitempty"` - ReadyTimeout string `json:"ready_timeout,omitempty"` - LogFile string `json:"log_file,omitempty"` - ForceArgs []string `json:"force_args,omitempty"` - Tools []string `json:"tools,omitempty"` + ReadyTCP string `json:"ready_tcp,omitempty"` + ReadyAfter string `json:"ready_after,omitempty"` + ReadyTimeout string `json:"ready_timeout,omitempty"` + LogFile string `json:"log_file,omitempty"` + ForceArgs []string `json:"force_args,omitempty"` + Tools []string `json:"tools,omitempty"` + ReadyPortFlag string `json:"ready_port_flag,omitempty"` } // SubParam is one structured input parameter (vs the old free-text field). @@ -734,12 +735,13 @@ func (s Submission) ToConfig() *scaffold.Config { } if sv := m.CLI.Service; sv != nil { method.CLI.Service = &scaffold.CLIService{ - ReadyTCP: sv.ReadyTCP, - ReadyAfter: sv.ReadyAfter, - ReadyTimeout: sv.ReadyTimeout, - LogFile: sv.LogFile, - ForceArgs: sv.ForceArgs, - Tools: sv.Tools, + ReadyTCP: sv.ReadyTCP, + ReadyAfter: sv.ReadyAfter, + ReadyTimeout: sv.ReadyTimeout, + LogFile: sv.LogFile, + ForceArgs: sv.ForceArgs, + Tools: sv.Tools, + ReadyPortFlag: sv.ReadyPortFlag, } } default: diff --git a/internal/scaffold/config.go b/internal/scaffold/config.go index 6e423b5..93e8555 100644 --- a/internal/scaffold/config.go +++ b/internal/scaffold/config.go @@ -944,6 +944,11 @@ type CLIService struct { // Tools (passthrough only): the args[0] names that start a server; any // other tool runs as a plain command. Required for a passthrough service. Tools []string `yaml:"tools,omitempty"` + // ReadyPortFlag (passthrough only): the argv flag that carries the + // server's TCP port, e.g. "--port". When a call passes it (`--port N` or + // `--port=N`), the server is ready once 127.0.0.1:N accepts (and the call + // fails fast if something already does) instead of after ReadyAfter. + ReadyPortFlag string `yaml:"ready_port_flag,omitempty"` } // ReadyAfterOrDefault / ReadyTimeoutOrDefault feed the generated Spec. @@ -1623,6 +1628,9 @@ func validateCLIService(i int, m Method, sv *CLIService) []error { if sv.ReadyTCP != "" || sv.LogFile != "" { errs = append(errs, fmt.Errorf("%s: ready_tcp/log_file need ${field} params, which a passthrough route does not have", at)) } + if f := sv.ReadyPortFlag; f != "" && (!strings.HasPrefix(f, "-") || strings.ContainsAny(f, "= ")) { + errs = append(errs, fmt.Errorf("%s.ready_port_flag %q must be a flag such as --port", at, f)) + } // Without an allowlist a caller could name the server by another path // (or wrap it in a shell) and start it daemonized, outside the service. if len(m.CLI.Tools) == 0 { @@ -1641,6 +1649,9 @@ func validateCLIService(i int, m Method, sv *CLIService) []error { if len(sv.Tools) > 0 { errs = append(errs, fmt.Errorf("%s.tools only applies to a passthrough route", at)) } + if sv.ReadyPortFlag != "" { + errs = append(errs, fmt.Errorf("%s.ready_port_flag only applies to a passthrough route (use ready_tcp)", at)) + } if sv.ReadyTCP != "" { host, port, err := net.SplitHostPort(sv.ReadyTCP) if err != nil || host == "" || port == "" { diff --git a/internal/scaffold/templates/main.go.tmpl b/internal/scaffold/templates/main.go.tmpl index 3874e06..890020c 100644 --- a/internal/scaffold/templates/main.go.tmpl +++ b/internal/scaffold/templates/main.go.tmpl @@ -733,7 +733,7 @@ func jsonScalar(v json.RawMessage) string { func registerHandlers(d *ipc.Dispatcher, r *backend.Runner, c *backend.CloudClient, version, backendURL string) { {{- range .Methods}} {{- if .CLI}} - d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{if .CLI.Tools}}, Tools: {{printf "%#v" .CLI.Tools}}{{end}}{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}}{{with .CLI.Service}}, Service: &backend.ServiceSpec{ReadyTCP: {{printf "%q" .ReadyTCP}}, ReadyAfter: dur({{printf "%q" .ReadyAfterOrDefault}}), ReadyTimeout: dur({{printf "%q" .ReadyTimeoutOrDefault}}), LogFile: {{printf "%q" .LogFile}}, ForceArgs: {{printf "%#v" .ForceArgs}}, Tools: {{printf "%#v" .Tools}}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} (local) + d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{if .CLI.Tools}}, Tools: {{printf "%#v" .CLI.Tools}}{{end}}{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}}{{with .CLI.Service}}, Service: &backend.ServiceSpec{ReadyTCP: {{printf "%q" .ReadyTCP}}, ReadyAfter: dur({{printf "%q" .ReadyAfterOrDefault}}), ReadyTimeout: dur({{printf "%q" .ReadyTimeoutOrDefault}}), LogFile: {{printf "%q" .LogFile}}, ForceArgs: {{printf "%#v" .ForceArgs}}, Tools: {{printf "%#v" .Tools}}, ReadyPortFlag: {{printf "%q" .ReadyPortFlag}}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} (local) {{- else}} d.Register("{{.Name}}", cloudHandler(c, "{{.HTTP.Verb}}", "{{.HTTP.Path}}", {{if eq .HTTP.Path $.ProvisionPath}}true{{else}}false{{end}}, dur("{{.TimeoutFor}}"))) // {{.Duration}} (cloud) {{- end}} @@ -827,7 +827,7 @@ func decodeCall(spec backend.Spec, raw json.RawMessage) (backend.Call, error) { {{- else}} func registerHandlers(d *ipc.Dispatcher, r *backend.Runner, version string) { {{- range .Methods}} - d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{if .CLI.Tools}}, Tools: {{printf "%#v" .CLI.Tools}}{{end}}{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}}{{with .CLI.Service}}, Service: &backend.ServiceSpec{ReadyTCP: {{printf "%q" .ReadyTCP}}, ReadyAfter: dur({{printf "%q" .ReadyAfterOrDefault}}), ReadyTimeout: dur({{printf "%q" .ReadyTimeoutOrDefault}}), LogFile: {{printf "%q" .LogFile}}, ForceArgs: {{printf "%#v" .ForceArgs}}, Tools: {{printf "%#v" .Tools}}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} + d.Register("{{.Name}}", forwardExec(r, backend.Spec{ {{if .CLI.Passthrough}}Passthrough: true{{if .CLI.Tools}}, Tools: {{printf "%#v" .CLI.Tools}}{{end}}{{else}}Args: {{printf "%#v" .CLI.Args}}, ParamsAsFlags: {{.CLI.ParamsAsFlags}}{{end}}{{with .CLI.Service}}, Service: &backend.ServiceSpec{ReadyTCP: {{printf "%q" .ReadyTCP}}, ReadyAfter: dur({{printf "%q" .ReadyAfterOrDefault}}), ReadyTimeout: dur({{printf "%q" .ReadyTimeoutOrDefault}}), LogFile: {{printf "%q" .LogFile}}, ForceArgs: {{printf "%#v" .ForceArgs}}, Tools: {{printf "%#v" .Tools}}, ReadyPortFlag: {{printf "%q" .ReadyPortFlag}}}{{end}} }, dur("{{.TimeoutFor}}"))) // {{.Duration}} {{- end}} d.Register("{{.Namespace}}.help", helpHandler(version)) } diff --git a/internal/scaffold/templates/service.go.tmpl b/internal/scaffold/templates/service.go.tmpl index d036bb0..d6f8225 100644 --- a/internal/scaffold/templates/service.go.tmpl +++ b/internal/scaffold/templates/service.go.tmpl @@ -37,6 +37,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "sync" "syscall" @@ -60,6 +61,12 @@ type ServiceSpec struct { LogFile string // the server's own log (${field}); its tail is returned on failure ForceArgs []string // appended after every other argument (last-wins flags) Tools []string // passthrough: the args[0] names that start a server + // ReadyPortFlag (passthrough): the argv flag carrying the server's TCP port + // (e.g. "--port"). When the call passes it, the server is ready once + // 127.0.0.1: accepts, not merely after ReadyAfter: a freshly staged + // binary's first launch can take seconds (macOS assesses it; Rosetta + // translates it) before it listens. + ReadyPortFlag string } // appliesTo reports whether this call starts a server. Every call of an @@ -105,12 +112,18 @@ func (r *Runner) startService(ctx context.Context, sv *ServiceSpec, args []strin return nil, err } readyAddr = a - // Something already answering there would make the readiness probe - // report success for a server that is about to fail to bind. - if dialOK(readyAddr, 300*time.Millisecond) { - return nil, fmt.Errorf("backend: %s is already accepting connections — a server is already running there; stop it or use another port", readyAddr) + } else if sv.ReadyPortFlag != "" { + if port := flagValue(call.Args, sv.ReadyPortFlag); port != "" { + if n, err := strconv.Atoi(port); err == nil && n > 0 && n < 65536 { + readyAddr = net.JoinHostPort("127.0.0.1", port) + } } } + // Something already answering there would make the readiness probe + // report success for a server that is about to fail to bind. + if readyAddr != "" && dialOK(readyAddr, 300*time.Millisecond) { + return nil, fmt.Errorf("backend: %s is already accepting connections — a server is already running there; stop it or use another port", readyAddr) + } if sv.LogFile != "" { if lf, err := substitute(sv.LogFile, call.Params); err == nil { logFile = lf @@ -324,6 +337,21 @@ func fileTail(path string, n int64) string { return strings.TrimSpace(string(b)) } +// flagValue returns the value of the last `flag value` or `flag=value` in +// args ("" if absent): servers such as redis-server take the last one. +func flagValue(args []string, flag string) string { + v := "" + for i, a := range args { + switch { + case a == flag && i+1 < len(args): + v = args[i+1] + case strings.HasPrefix(a, flag+"="): + v = strings.TrimPrefix(a, flag+"=") + } + } + return v +} + func dialOK(addr string, timeout time.Duration) bool { c, err := net.DialTimeout("tcp", addr, timeout) if err != nil { diff --git a/internal/scaffold/zz_cli_service_config_test.go b/internal/scaffold/zz_cli_service_config_test.go index f451fd8..79088d4 100644 --- a/internal/scaffold/zz_cli_service_config_test.go +++ b/internal/scaffold/zz_cli_service_config_test.go @@ -36,6 +36,9 @@ methods: {"bad duration", ` cli: {args: ["x"], service: {ready_after: "soon"}}`, "not a positive Go duration"}, {"ready_timeout past method timeout", ` cli: {args: ["x"], service: {ready_timeout: "90s"}}`, "must be below the method timeout"}, {"service.tools on enumerated", ` cli: {args: ["x"], service: {tools: ["a"]}}`, "tools only applies to a passthrough route"}, + {"ok ready_port_flag", ` cli: {passthrough: true, tools: ["a"], service: {tools: ["a"], ready_port_flag: "--port"}}`, ""}, + {"ready_port_flag not a flag", ` cli: {passthrough: true, tools: ["a"], service: {tools: ["a"], ready_port_flag: "port"}}`, "must be a flag such as --port"}, + {"ready_port_flag on enumerated", ` cli: {args: ["x"], service: {ready_port_flag: "--port"}}`, "ready_port_flag only applies to a passthrough route"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/scaffold/zz_cli_service_lifetime_test.go b/internal/scaffold/zz_cli_service_lifetime_test.go index 9fe5b19..22e3ca5 100644 --- a/internal/scaffold/zz_cli_service_lifetime_test.go +++ b/internal/scaffold/zz_cli_service_lifetime_test.go @@ -5,10 +5,12 @@ package scaffold import ( "bufio" "encoding/json" + "flag" "fmt" "net" "os" "os/exec" + "os/signal" "path/filepath" "strconv" "strings" @@ -19,6 +21,57 @@ import ( "github.com/pilot-protocol/app-store/pkg/ipc" ) +// slowListenerEnv switches the test binary into a server that starts +// listening only after a delay, as a freshly staged binary does on its first +// launch (macOS assesses it, Rosetta translates it): TestHelperSlowListener. +const slowListenerEnv = "SCAFFOLD_TEST_SLOW_LISTENER" + +// TestHelperSlowListener is not a test: it is the `listen` tool of +// TestCLIServiceLifetime and skips unless slowListenerEnv is set. Args (after +// --): --port P (or --port=P) --delay D --pidfile F; it listens on +// 127.0.0.1:P after D and runs until SIGTERM. +func TestHelperSlowListener(t *testing.T) { + if os.Getenv(slowListenerEnv) == "" { + t.Skip("helper process for TestCLIServiceLifetime") + } + args := flag.Args() + val := func(name string) string { + v := "" + for i, a := range args { + if a == name && i+1 < len(args) { + v = args[i+1] + } else if strings.HasPrefix(a, name+"=") { + v = strings.TrimPrefix(a, name+"=") + } + } + return v + } + sig := make(chan os.Signal, 1) + signal.Notify(sig, syscall.SIGTERM) + _ = os.WriteFile(val("--pidfile"), []byte(strconv.Itoa(os.Getpid())), 0o644) + delay, _ := time.ParseDuration(val("--delay")) + select { + case <-sig: + os.Exit(0) + case <-time.After(delay): + } + ln, err := net.Listen("tcp", "127.0.0.1:"+val("--port")) + if err != nil { + os.Exit(3) + } + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + _ = c.Close() + } + }() + <-sig + os.Exit(0) +} + // TestCLIServiceLifetime: a server started by a cli.service method must not // outlive the adapter, however the adapter goes away. Found with io.pilot.redis // 8.6.2: redis.start ran `redis-server --daemonize yes`, whose server setsid()s @@ -55,6 +108,7 @@ func TestCLIServiceLifetime(t *testing.T) { "case \"$1\" in\n" + " serve) echo \"$$ $*\" > \"$2\"; trap 'echo term > \"$2.term\"; kill $! 2>/dev/null; exit 0' TERM; sleep 300 & wait;;\n" + " quick) echo quick-out; exit 3;;\n" + + " listen) shift; exec env " + slowListenerEnv + "=1 '" + os.Args[0] + "' -test.run='^TestHelperSlowListener$' -- \"$@\";;\n" + " *) echo '{}';;\n" + "esac\n" if err := os.WriteFile(tool, []byte(script), 0o755); err != nil { @@ -83,8 +137,8 @@ methods: summary: "Passthrough." cli: passthrough: true - tools: ["serve", "quick"] - service: {tools: ["serve"], ready_after: "300ms", force_args: ["--daemonize", "no"]} + tools: ["serve", "quick", "listen"] + service: {tools: ["serve", "listen"], ready_after: "300ms", ready_port_flag: "--port", force_args: ["--daemonize", "no"]} `) proj := filepath.Join(root, "proj") if _, err := Generate(cfg, proj); err != nil { @@ -328,6 +382,50 @@ methods: waitFor(t, func() bool { return !procAlive(srv) }, 12*time.Second, "never-ready server to be stopped") }) + // Passthrough readiness follows the server's --port: a server that takes + // longer than ready_after to listen (a first launch) is only reported + // started once 127.0.0.1: accepts, and a busy port fails fast. + t.Run("passthrough-ready-port", func(t *testing.T) { + dir := shortDir(t) + sock := filepath.Join(dir, "app.sock") + spawn(t, sock) + ln, _ := net.Listen("tcp", "127.0.0.1:0") + port := strconv.Itoa(ln.Addr().(*net.TCPAddr).Port) + ln.Close() + for i, portArgs := range [][]string{{"--port", port}, {"--port=" + port}} { + pf := filepath.Join(dir, fmt.Sprintf("l%d.pid", i)) + args := append([]string{"listen", "--delay", "1500ms", "--pidfile", pf}, portArgs...) + t0 := time.Now() + res, err := call(sock, "svctool.exec", map[string]any{"args": args}) + if err != nil || res["exit"] != float64(0) { + t.Fatalf("listen %v = %v, %v", portArgs, res, err) + } + if took := time.Since(t0); took < 1400*time.Millisecond { + t.Errorf("listen %v returned after %s, before the server listened (1.5s)", portArgs, took) + } + if c, err := net.DialTimeout("tcp", "127.0.0.1:"+port, time.Second); err != nil { + t.Errorf("listen %v reported started but 127.0.0.1:%s refuses: %v", portArgs, port, err) + } else { + c.Close() + } + if !strings.Contains(fmt.Sprint(res["stdout"]), "accepting connections on 127.0.0.1:"+port) { + t.Errorf("reply %v does not name the address it waited for", res) + } + // The same port again: already accepting, so nothing starts. + pf2 := filepath.Join(dir, fmt.Sprintf("l%d-busy.pid", i)) + if _, err := call(sock, "svctool.exec", map[string]any{"args": []string{"listen", "--port", port, "--delay", "0s", "--pidfile", pf2}}); err == nil || !strings.Contains(err.Error(), "already accepting connections") { + t.Errorf("second listen on %s: err = %v, want already-accepting refusal", port, err) + } + if _, err := os.Stat(pf2); err == nil { + t.Errorf("a server was started on the busy port %s", port) + } + // Stop this one before the next form reuses the port. + pid, _ := readPid(t, pf) + _ = syscall.Kill(pid, syscall.SIGTERM) + waitFor(t, func() bool { return !procAlive(pid) }, 5*time.Second, "listener to stop") + } + }) + // Several services at once all stop on SIGTERM, within the grace. t.Run("sigterm/many", func(t *testing.T) { dir := shortDir(t) diff --git a/submissions/io.pilot.redis/submission.json b/submissions/io.pilot.redis/submission.json index e9dcb57..f3b724f 100644 --- a/submissions/io.pilot.redis/submission.json +++ b/submissions/io.pilot.redis/submission.json @@ -205,7 +205,7 @@ }, { "name": "redis.exec", - "description": "Run any bundled Redis tool with a verbatim argv — the full surface beyond the curated methods. Payload is {\"args\":[, ...]} where the first element is the tool (redis-cli, redis-server, redis-benchmark, redis-check-rdb, redis-check-aof, redis-sentinel) and the rest are its args; optional {\"stdin\":\"...\"} is piped to it. This is how you run ANY Redis command: {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"LPUSH\",\"mylist\",\"a\",\"b\",\"c\"]}, {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"-n\",\"1\",\"HSET\",\"h\",\"f\",\"v\"]}, a pipelined script via stdin {\"args\":[\"redis-cli\",\"-p\",\"6399\"],\"stdin\":\"SET a 1\\nINCR a\\nGET a\"}, or a benchmark {\"args\":[\"redis-benchmark\",\"-p\",\"6399\",\"-n\",\"1000\",\"-q\"]}. The REDISCLI_AUTH env var is passed through for password auth. args[0] must be one of the six bundled tools. A redis-server or redis-sentinel started here runs like redis.start: under this app, kept in the foreground (--daemonize no is appended), returned once it has stayed up for 1s (a --help/--version/config-error run just returns its output), and stopped when the app stops.", + "description": "Run any bundled Redis tool with a verbatim argv — the full surface beyond the curated methods. Payload is {\"args\":[, ...]} where the first element is the tool (redis-cli, redis-server, redis-benchmark, redis-check-rdb, redis-check-aof, redis-sentinel) and the rest are its args; optional {\"stdin\":\"...\"} is piped to it. This is how you run ANY Redis command: {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"LPUSH\",\"mylist\",\"a\",\"b\",\"c\"]}, {\"args\":[\"redis-cli\",\"-p\",\"6399\",\"-n\",\"1\",\"HSET\",\"h\",\"f\",\"v\"]}, a pipelined script via stdin {\"args\":[\"redis-cli\",\"-p\",\"6399\"],\"stdin\":\"SET a 1\\nINCR a\\nGET a\"}, or a benchmark {\"args\":[\"redis-benchmark\",\"-p\",\"6399\",\"-n\",\"1000\",\"-q\"]}. The REDISCLI_AUTH env var is passed through for password auth. args[0] must be one of the six bundled tools. A redis-server or redis-sentinel started here runs like redis.start: under this app, kept in the foreground (--daemonize no is appended), returned once it accepts connections on 127.0.0.1:<--port> (or, with no --port, once it has stayed up for 1s; a --help/--version/config-error run just returns its output), and stopped when the app stops.", "latency": "med", "params": [ { @@ -227,6 +227,7 @@ "service": { "tools": ["redis-server", "redis-sentinel"], "force_args": ["--daemonize", "no"], + "ready_port_flag": "--port", "ready_after": "1s", "ready_timeout": "30s" }