Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
.DS_Store
dist/
# Anchored to the repository root. Written as a bare `dist/` it matched at any
# depth, and because excluding a directory stops git from descending into it, the
# `!frontend/dist/.keep` exception two lines down could never take effect — so
# the .keep that manifest_embed.go's `go:embed all:frontend/dist` needs was
# silently absent from every clone.
/dist/
release/
frontend/node_modules/
frontend/dist/*
Expand Down
8 changes: 7 additions & 1 deletion frontend/.gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
node_modules/
dist/
# The bundle is ignored but dist/.keep is not: manifest_embed.go carries a
# `go:embed all:frontend/dist`, which does not compile when the directory is
# absent, so a fresh clone could not `go build` or `go vet` before Vite had run.
# Listed per-entry rather than as `dist/` because excluding the directory itself
# stops git descending into it, which would make the exception unreachable.
dist/*
!dist/.keep
coverage/
test-results/
playwright-report/
Expand Down
Empty file added frontend/dist/.keep
Empty file.
19 changes: 17 additions & 2 deletions internal/process/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"os/exec"
"sort"
"strings"
"sync"
"time"
)

Expand All @@ -32,6 +33,12 @@ type Output struct {
Text string `json:"text,omitempty"`
}

// OutputListener receives each accepted chunk of a command's output.
//
// Calls are serialised: stdout and stderr are copied by separate goroutines, so
// a listener would otherwise be entered concurrently, and every caller would
// have to synchronise on its own. Implementations may append to a slice or write
// to a channel without locking.
type OutputListener func(Output)

type StreamingRunner interface {
Expand Down Expand Up @@ -90,8 +97,9 @@ func (r OSRunner) RunWithOutput(ctx context.Context, argv []string, overrides ma
command.Env = mergeEnvironment(r.Env, overrides)
stdout := &boundedBuffer{limit: MaxOutputBytes}
stderr := &boundedBuffer{limit: MaxOutputBytes}
command.Stdout = &streamWriter{stream: "stdout", buffer: stdout, listener: listener}
command.Stderr = &streamWriter{stream: "stderr", buffer: stderr, listener: listener}
var streamLock sync.Mutex
command.Stdout = &streamWriter{stream: "stdout", buffer: stdout, listener: listener, mu: &streamLock}
command.Stderr = &streamWriter{stream: "stderr", buffer: stderr, listener: listener, mu: &streamLock}
err := command.Run()
result.Stdout = stdout.String()
result.Stderr = stderr.String()
Expand All @@ -118,9 +126,16 @@ type streamWriter struct {
stream string
buffer *boundedBuffer
listener OutputListener
// Shared by the stdout and stderr writers of one command, so a listener sees
// one chunk at a time. Each writer has its own buffer, but a lock per writer
// would not serialise anything — the two goroutines would take different
// locks and still enter the listener together.
mu *sync.Mutex
}

func (w *streamWriter) Write(data []byte) (int, error) {
w.mu.Lock()
defer w.mu.Unlock()
before := w.buffer.buffer.Len()
n, err := w.buffer.Write(data)
accepted := w.buffer.buffer.Len() - before
Expand Down
51 changes: 50 additions & 1 deletion internal/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ func TestProcessHelper(t *testing.T) {
// kills this process, so the sleep never runs to completion.
<-time.After(10 * time.Second)
}
// Interleaves both streams so the runner's stdout and stderr copiers are
// active at the same time. Real installs look like this — npm reports progress
// on stderr while printing results on stdout.
if os.Getenv("ONEAGENT_PROCESS_BOTH_STREAMS") == "1" {
for index := 0; index < 50; index++ {
os.Stdout.WriteString("o")
os.Stderr.WriteString("e")
}
os.Exit(0)
}
os.Stdout.WriteString(os.Getenv("ONEAGENT_PROCESS_VALUE"))
os.Exit(0)
}
Expand All @@ -38,7 +48,16 @@ func helperRunner(t *testing.T) OSRunner {
if err != nil {
t.Fatal(err)
}
runner := New(map[string]string{"ONEAGENT_PROCESS_HELPER": "1"})
/* These cases re-exec the test binary as the helper process, so under `go test
-cover` the child inherits the coverage instrumentation. Without a place to
write its profile it prints "warning: GOCOVERDIR not set" to stderr, which
lands in the captured output and in the listener — failing assertions about
what the command produced for a reason that has nothing to do with the
runner. Giving the child a scratch directory keeps its stderr its own. */
runner := New(map[string]string{
"ONEAGENT_PROCESS_HELPER": "1",
"GOCOVERDIR": t.TempDir(),
})
runner.Lookup = func(command string) (string, bool) {
if command == "helper" {
return path, true
Expand Down Expand Up @@ -75,6 +94,36 @@ func TestOSRunnerStreamsOutputWithoutChangingResult(t *testing.T) {
}
}

// The listener here appends without locking, exactly as the install runtime's
// does (internal/app/install.go redacts and forwards). stdout and stderr are
// copied by separate goroutines, so without serialisation inside the runner this
// is a data race in production, not just in a test — it surfaces whenever a
// command writes to both streams, which npm does on every install.
func TestOSRunnerSerialisesListenerAcrossStreams(t *testing.T) {
runner := helperRunner(t)
outputs := make([]Output, 0)
result, err := runner.RunWithOutput(context.Background(), []string{os.Args[0], "-test.run=TestProcessHelper"}, map[string]string{
"ONEAGENT_PROCESS_BOTH_STREAMS": "1",
}, helperTimeout, func(output Output) { outputs = append(outputs, output) })
if err != nil || result.ExitCode != 0 {
t.Fatalf("interleaved process result = %#v, err=%v", result, err)
}
var stdout, stderr int
for _, output := range outputs {
switch output.Stream {
case "stdout":
stdout += len(output.Text)
case "stderr":
stderr += len(output.Text)
}
}
// Both streams have to reach the listener; asserting only the total would
// pass even if one stream's chunks were being dropped.
if stdout != 50 || stderr != 50 {
t.Fatalf("streamed %d stdout and %d stderr bytes, want 50 of each", stdout, stderr)
}
}

func TestOSRunnerReturnsExitCodeAndCapturesOutput(t *testing.T) {
runner := helperRunner(t)
result, err := runner.Run(context.Background(), []string{os.Args[0], "-test.run=TestProcessHelper"}, map[string]string{
Expand Down
7 changes: 7 additions & 0 deletions site/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ export default defineConfig({
site,
base,
output: "static",
// Chinese stays unprefixed so every published URL keeps working; English is
// additive under /en/.
i18n: {
defaultLocale: "zh-CN",
locales: ["zh-CN", "en"],
routing: { prefixDefaultLocale: false },
},
integrations: [sitemap()],
build: {
assets: "_assets",
Expand Down
Loading