Skip to content

[Compiler] Introduce one bounded subprocess supervisor for all external tools #685

Description

@cssbruno

[Compiler] Introduce one bounded subprocess supervisor for all external tools

Priority

High — reliability, security hardening, and cross-platform consistency

Context

GOWDK invokes external processes from multiple compiler, generator, tooling, and test paths. Current examples include:

  • Go package inspection and go list;
  • build-data helper execution;
  • executable configuration and addon helpers;
  • generated application and binary compilation;
  • WASM compilation;
  • Tailwind and other external CSS tooling;
  • contract and type inspection helpers;
  • audit/test subprocesses;
  • dev-server child processes;
  • doctor and tool-version checks;
  • playground execution.

These call sites currently use a mixture of exec.Command, exec.CommandContext, direct Output, custom stderr buffers, and command-specific timeout or cleanup behavior.

Issue #672 correctly scopes a versioned executable-addon host, but the broader compiler still lacks one process execution contract.

Problem

Independent subprocess implementations create recurring defects and inconsistent behavior:

  1. Some commands can run indefinitely while others are cancellable.
  2. Output capture is often unbounded or formatted differently.
  3. Environment inheritance differs by call site and can expose runtime secrets to compilation helpers unnecessarily.
  4. Child/descendant cleanup differs between Unix and Windows.
  5. Errors expose inconsistent command, exit, signal, and stderr context.
  6. Secret redaction is not guaranteed at the process boundary.
  7. Tool cache, module, workspace, proxy, and network policies are duplicated or implicit.
  8. Dev, CI, and one-shot commands can react differently to the same external-tool failure.
  9. New integrations are likely to copy an existing partial implementation.

Goal

Create one internal subprocess supervision package used by all compiler-owned external command execution. The package should make execution bounded, cancellable, observable, testable, and cross-platform by default.

Proposed architecture

Introduce an internal package with a narrow API, for example:

package subprocess

type Spec struct {
    Name            string
    Args            []string
    Dir             string
    EnvPolicy       EnvPolicy
    ExtraEnv        []string
    Timeout         time.Duration
    MaxStdoutBytes  int64
    MaxStderrBytes  int64
    Stdin           io.Reader
    KillProcessTree bool
    Redactor        Redactor
    Phase           string
    Source          source.SourceSpan
}

type Result struct {
    ExitCode        int
    Signal          string
    Duration        time.Duration
    Stdout          []byte
    Stderr          []byte
    StdoutTruncated bool
    StderrTruncated bool
    TimedOut        bool
    Cancelled       bool
}

func Run(ctx context.Context, spec Spec) (Result, error)

The exact API may differ, but call sites should not manage raw os/exec.Cmd lifecycle directly unless a documented low-level exception is approved.

Required behavior

Context and deadlines

  • Every invocation accepts a parent context.
  • Each command category has a documented default timeout.
  • Commands may explicitly disable the timeout only through a reviewed exceptional path.
  • Timeout and cancellation produce distinguishable structured errors.

Process-tree management

  • Unix execution should use an isolated process group when descendant termination is required.
  • Windows execution should use the best supported job/process-tree mechanism for the CLI's supported versions.
  • Cancellation should not leave go, compiler, test, Tailwind, generated binaries, or helper descendants running.
  • Cleanup must be idempotent and safe when the process exits concurrently.

Bounded I/O

  • Stdout and stderr have per-command-category defaults.
  • Truncation is explicit in the result and diagnostic.
  • Streaming mode should be available for dev/test commands while preserving bounded retained diagnostics.
  • Binary result protocols should use dedicated files/descriptors rather than competing with human logs.

Environment policy

Define named policies rather than ad hoc os.Environ() use, such as:

  • InheritDeveloperEnvironment for explicitly trusted local commands;
  • CompilerEnvironment with runtime/deployment secrets removed;
  • GoToolEnvironment with deliberate GOWORK, GOPROXY, GOSUMDB, cache, tags, and target settings;
  • IsolatedEnvironment for playground/hosted execution.

Environment keys and values included in diagnostics must be allowlisted and secret-redacted.

Error model

Return a typed error containing:

  • logical operation and phase;
  • executable and safely rendered arguments;
  • working directory;
  • exit code or terminating signal;
  • timeout/cancellation state;
  • bounded, redacted stderr/stdout excerpts;
  • source span or owning config field when available;
  • remediation hint where the caller provides one.

Do not require callers to parse error-message substrings to classify failures.

Observability

Emit structured lifecycle events suitable for build reports and timings:

process.started
process.completed
process.failed
process.timed_out
process.cancelled
process.output_truncated

Events must avoid logging secrets, bearer credentials, private environment values, or unsafe command arguments.

Migration plan

  1. Add the supervisor and platform-specific process-tree tests.
  2. Migrate build-data execution first because it currently lacks cancellation and has a result-protocol concern.
  3. Migrate executable config/addon execution in coordination with Unify gowdk.config.go loading under one explicit execution model #665 and Version and harden the executable addon bridge #672.
  4. Migrate Go inspection, generated-app compilation, WASM, Tailwind, audit/test, doctor, and dev child processes.
  5. Add a repository check that rejects new direct exec.Command/exec.CommandContext usage outside the supervisor and an explicit allowlist.
  6. Remove superseded command-specific capture, redaction, timeout, and cleanup code.

Non-goals

  • Making every command use the same timeout or output limit.
  • Treating trusted local build code as fully sandboxed.
  • Hiding raw tool output when the user explicitly requests verbose/debug output.
  • Replacing Go's own module and build cache behavior.
  • Moving hosted playground isolation into the ordinary local-build path.

Test plan

The supervisor should have deterministic tests for:

  • successful command with stdout/stderr;
  • non-zero exit;
  • signal termination;
  • timeout;
  • caller cancellation;
  • output truncation at exact boundaries;
  • stdin delivery;
  • environment allow/deny behavior;
  • redaction of secret-like arguments and environment values;
  • invalid working directory and missing executable;
  • descendant process cleanup;
  • cancellation race with normal exit;
  • streaming plus bounded retained output;
  • Unix and Windows behavior;
  • commands containing spaces and non-ASCII paths;
  • result stability under concurrent invocations.

Add integration coverage for representative consumers:

  • go list/binding inspection;
  • build-data helper;
  • external addon host;
  • generated binary build;
  • WASM build;
  • Tailwind invocation;
  • dev child restart;
  • gowdk test external browser command.

Acceptance criteria

  • One internal package owns compiler subprocess lifecycle, limits, cancellation, redaction, and process-tree termination.
  • All production call sites use it or carry a narrowly documented exception.
  • No default compiler-owned command can wait forever without an explicit policy decision.
  • Stdout and stderr retention is bounded by default.
  • Runtime/deployment secrets are not inherited by compiler helpers unless explicitly required.
  • Typed errors replace message-substring parsing for process classification.
  • Build reports/timings can record command outcomes consistently.
  • Cancellation removes descendant processes on supported Unix and Windows paths.
  • A CI check prevents new unmanaged os/exec call sites.
  • Existing command UX remains understandable and preserves opt-in verbose output.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions