mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-23 11:53:08 -05:00
## Description
Make ODS reuse shared development infrastructure by default, with
`--isolated` for separate datasets.
- Use the same service specifications and launchers for `stack`, `web
dev`, and `backend` commands. Remove the duplicate `stack
--frontend-only` option.
- Enforce project ownership across checkouts and resolve database
commands to the actual managed database.
- Stop services before pending migrations, preserve them when already at
head, and reject manual migrations beneath managed services.
- Harden startup cancellation, shutdown grace, descendant cleanup,
terminal input, and frontend API validation. Preserve Windows foreground
commands.
- Consolidate environment helpers, protect symlinked environment files,
propagate restore errors, and update help and documentation.
This PR contains only ODS and its development-stack skill. It does not
include the plugins implementation.
## How Has This Been Tested?
- Targeted command and regression tests using fake tools and disposable
Git repositories.
- Race tests for runtime targeting, ownership, migration ordering,
process cleanup, and terminal input.
- `go vet ./...`, pre-commit checks, native builds, and Windows
cross-builds.
- The full Go suite was run. Its remaining failures are the existing
macOS devcontainer SSH-mount fixture expectations in
`TestDevcontainerCommands` and `TestRunDevRebuild`.
No real database migrations, restores, or development-stack restarts
ran. Native Windows execution remains untested.
## Additional Options
- [ ] [Optional] Please cherry-pick this PR to the latest release
version.
- [x] [Optional] Override Linear Check
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
ODS now shares development infrastructure across worktrees by default,
with `--isolated` for separate datasets. `stack`, `web dev`, and
`backend` use the same service specs and launchers, and the duplicate
`stack --frontend-only` option is removed.
- Direct launches run the same dependency-prepare step as stack startup,
so `backend` and `web dev` work with fresh or stale Python environments.
- A stopped preview discards its former project state before retargeting
to another API owner.
- Enforces project ownership across checkouts and resolves database
commands to the managed database. New tests reject conflicting targets
before any mutation.
- Stops services before pending migrations, preserves them when already
at head, and rejects manual migrations beneath managed services.
Migration probe failures fail closed.
- Hardens startup cancellation, shutdown grace, and descendant cleanup;
ODS keeps the terminal foreground group so Ctrl-C still honors shutdown
timeouts, and tmux pane identifiers no longer change service identity.
- Database passwords reach tools through the child environment only,
never in process arguments; Windows foreground commands are preserved.
- Consolidates environment helpers, protects symlinked env files,
propagates restore errors, and refreshes help and documentation. Test
commits exercise the new migration-target and preview-launch paths.
<sup>Written for commit 14500cb051.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/onyx-dot-app/onyx/pull/16246?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/onyx-dot-app/onyx/16246?returnTo=https%3A%2F%2Fgithub.com%2Fonyx-dot-app%2Fonyx%2Fpull%2F16246&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added the global `--isolated` option for separate development stacks;
shared infrastructure is the default, with clearer project ownership and
status information.
* Added `ods web dev` previews that can use a healthy managed API on
macOS and Linux, plus foreground startup on Windows.
* Expanded command guidance for development workflows, database
targeting, and dump formats.
* **Bug Fixes**
* Pending migrations now stop managed application services first;
healthy services remain running when no migrations are needed.
* Database operations verify project ownership and target selection
before making changes. Restore errors now fail the command, SQL restores
stop on errors, and temporary files are cleaned up after failures.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
323 lines
9.4 KiB
Go
323 lines
9.4 KiB
Go
package cmd
|
|
|
|
import (
|
|
"maps"
|
|
"os"
|
|
"path/filepath"
|
|
"slices"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/onyx-dot-app/onyx/tools/ods/internal/devstack"
|
|
)
|
|
|
|
func TestUpsertEnvFile_createsFileWhenMissing(t *testing.T) {
|
|
dir := t.TempDir()
|
|
envPath := filepath.Join(dir, ".env")
|
|
|
|
err := devstack.UpsertEnvFile(envPath, map[string]string{
|
|
"FOO": "bar",
|
|
"BAZ": "qux",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
|
|
data, err := os.ReadFile(envPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to read file: %v", err)
|
|
}
|
|
|
|
content := string(data)
|
|
if !strings.Contains(content, "FOO=bar") {
|
|
t.Errorf("expected FOO=bar in output, got:\n%s", content)
|
|
}
|
|
if !strings.Contains(content, "BAZ=qux") {
|
|
t.Errorf("expected BAZ=qux in output, got:\n%s", content)
|
|
}
|
|
}
|
|
|
|
func TestUpsertEnvFile_upsertsExistingKeys(t *testing.T) {
|
|
dir := t.TempDir()
|
|
envPath := filepath.Join(dir, ".env")
|
|
|
|
initial := "FOO=old\nOTHER=keep\n"
|
|
if err := os.WriteFile(envPath, []byte(initial), 0644); err != nil {
|
|
t.Fatalf("failed to write initial file: %v", err)
|
|
}
|
|
|
|
err := devstack.UpsertEnvFile(envPath, map[string]string{
|
|
"FOO": "new",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
|
|
data, err := os.ReadFile(envPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to read file: %v", err)
|
|
}
|
|
|
|
content := string(data)
|
|
if !strings.Contains(content, "FOO=new") {
|
|
t.Errorf("expected FOO=new, got:\n%s", content)
|
|
}
|
|
if strings.Contains(content, "FOO=old") {
|
|
t.Errorf("old value FOO=old should be replaced, got:\n%s", content)
|
|
}
|
|
if !strings.Contains(content, "OTHER=keep") {
|
|
t.Errorf("OTHER=keep should be preserved, got:\n%s", content)
|
|
}
|
|
}
|
|
|
|
func TestUpsertEnvFile_appendsNewKeys(t *testing.T) {
|
|
dir := t.TempDir()
|
|
envPath := filepath.Join(dir, ".env")
|
|
|
|
initial := "EXISTING=value\n"
|
|
if err := os.WriteFile(envPath, []byte(initial), 0644); err != nil {
|
|
t.Fatalf("failed to write initial file: %v", err)
|
|
}
|
|
|
|
err := devstack.UpsertEnvFile(envPath, map[string]string{
|
|
"NEW_KEY": "new_value",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
|
|
data, err := os.ReadFile(envPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to read file: %v", err)
|
|
}
|
|
|
|
content := string(data)
|
|
if !strings.Contains(content, "EXISTING=value") {
|
|
t.Errorf("EXISTING=value should be preserved, got:\n%s", content)
|
|
}
|
|
if !strings.Contains(content, "NEW_KEY=new_value") {
|
|
t.Errorf("expected NEW_KEY=new_value appended, got:\n%s", content)
|
|
}
|
|
}
|
|
|
|
func TestUpsertEnvFile_doesNotDuplicateOnRepeatedCalls(t *testing.T) {
|
|
dir := t.TempDir()
|
|
envPath := filepath.Join(dir, ".env")
|
|
|
|
values := map[string]string{
|
|
"PORT": "5432",
|
|
}
|
|
|
|
for i := 0; i < 5; i++ {
|
|
if err := devstack.UpsertEnvFile(envPath, values); err != nil {
|
|
t.Fatalf("call %d: unexpected error: %v", i, err)
|
|
}
|
|
}
|
|
|
|
data, err := os.ReadFile(envPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to read file: %v", err)
|
|
}
|
|
|
|
count := strings.Count(string(data), "PORT=5432")
|
|
if count != 1 {
|
|
t.Errorf("expected exactly 1 occurrence of PORT=5432 after 5 calls, got %d:\n%s", count, string(data))
|
|
}
|
|
}
|
|
|
|
func TestUpsertEnvFile_doesNotMatchCommentedOutKeys(t *testing.T) {
|
|
dir := t.TempDir()
|
|
envPath := filepath.Join(dir, ".env")
|
|
|
|
initial := "# FOO=old_commented\nBAR=keep\n"
|
|
if err := os.WriteFile(envPath, []byte(initial), 0644); err != nil {
|
|
t.Fatalf("failed to write initial file: %v", err)
|
|
}
|
|
|
|
err := devstack.UpsertEnvFile(envPath, map[string]string{
|
|
"FOO": "new",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("unexpected error: %v", err)
|
|
}
|
|
|
|
data, err := os.ReadFile(envPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to read file: %v", err)
|
|
}
|
|
|
|
content := string(data)
|
|
if !strings.Contains(content, "# FOO=old_commented") {
|
|
t.Errorf("comment line should be preserved, got:\n%s", content)
|
|
}
|
|
if !strings.Contains(content, "FOO=new") {
|
|
t.Errorf("expected FOO=new appended, got:\n%s", content)
|
|
}
|
|
}
|
|
|
|
func TestUpsertEnvFile_overwritesWithNewValue(t *testing.T) {
|
|
dir := t.TempDir()
|
|
envPath := filepath.Join(dir, ".env")
|
|
|
|
if err := devstack.UpsertEnvFile(envPath, map[string]string{"PORT": "5432"}); err != nil {
|
|
t.Fatalf("first call: %v", err)
|
|
}
|
|
if err := devstack.UpsertEnvFile(envPath, map[string]string{"PORT": "15432"}); err != nil {
|
|
t.Fatalf("second call: %v", err)
|
|
}
|
|
|
|
data, err := os.ReadFile(envPath)
|
|
if err != nil {
|
|
t.Fatalf("failed to read file: %v", err)
|
|
}
|
|
|
|
content := string(data)
|
|
if strings.Contains(content, "PORT=5432") {
|
|
t.Errorf("old value PORT=5432 should be gone, got:\n%s", content)
|
|
}
|
|
if !strings.Contains(content, "PORT=15432") {
|
|
t.Errorf("expected PORT=15432, got:\n%s", content)
|
|
}
|
|
if strings.Count(content, "PORT=") != 1 {
|
|
t.Errorf("expected exactly 1 PORT= line, got:\n%s", content)
|
|
}
|
|
}
|
|
|
|
func TestQueryContainerPorts_usesRunningContainersOnly(t *testing.T) {
|
|
bin := composeFakeBin(t)
|
|
composeFakeTool(t, bin, "docker", `case "$2" in *-relational_db-1|*-object-store-1) echo "0.0.0.0:3$3" ;; *) exit 1 ;; esac`)
|
|
logs := composeCaptureLog(t)
|
|
|
|
resolved := queryContainerPorts("proj")
|
|
|
|
wantEnv := map[string]string{
|
|
"POSTGRES_HOST_PORT": "35432",
|
|
"OBJECT_STORE_HOST_PORT": "38333",
|
|
}
|
|
if got := resolved.ComposeEnv(); !maps.Equal(got, wantEnv) {
|
|
t.Fatalf("expected %v, got %v", wantEnv, got)
|
|
}
|
|
wantCalls := []string{
|
|
"port proj-relational_db-1 5432",
|
|
"port proj-cache-1 6379",
|
|
"port proj-opensearch-1 9200",
|
|
"port proj-inference_model_server-1 9000",
|
|
"port proj-object-store-1 8333",
|
|
"port proj-code-interpreter-1 8000",
|
|
}
|
|
if got := composeCalls(t, bin, "docker"); !slices.Equal(got, wantCalls) {
|
|
t.Fatalf("expected %q, got %q", wantCalls, got)
|
|
}
|
|
wantLogs := "level=warning msg=cache: container not running, skipping getting its port.\n" +
|
|
"level=warning msg=opensearch: container not running, skipping getting its port.\n" +
|
|
"level=warning msg=inference_model_server: container not running, skipping getting its port.\n" +
|
|
"level=warning msg=code-interpreter: container not running, skipping getting its port.\n"
|
|
if logs.String() != wantLogs {
|
|
t.Fatalf("expected logs %q, got %q", wantLogs, logs.String())
|
|
}
|
|
}
|
|
|
|
// composeAppEnv is the .vscode/.env content for containers whose host ports
|
|
// are the container ports prefixed with 1.
|
|
var composeAppEnv = map[string]string{
|
|
"POSTGRES_PORT": "15432",
|
|
"REDIS_PORT": "16379",
|
|
"OPENSEARCH_REST_API_PORT": "19200",
|
|
"MODEL_SERVER_PORT": "19000",
|
|
"S3_ENDPOINT_URL": "http://localhost:18333",
|
|
"CODE_INTERPRETER_BASE_URL": "http://localhost:18000",
|
|
}
|
|
|
|
func composeEnvDocker(t *testing.T) string {
|
|
t.Helper()
|
|
bin := composeFakeBin(t)
|
|
composeFakeTool(t, bin, "docker", `echo "0.0.0.0:1$3"`)
|
|
return bin
|
|
}
|
|
|
|
func TestEnvCommand_updatesVSCodeEnvInPlace(t *testing.T) {
|
|
bin := composeEnvDocker(t)
|
|
root := composeRepo(t)
|
|
envPath := filepath.Join(root, ".vscode", ".env")
|
|
writeFile(t, envPath, "CUSTOM_SETTING=x\nPOSTGRES_PORT=1\nS3_LEGACY_ENDPOINT_URL=http://localhost:9005\n")
|
|
|
|
command := NewEnvCommand()
|
|
command.SetArgs([]string{})
|
|
if err := command.Execute(); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
content := composeReadFile(t, envPath)
|
|
if !strings.HasPrefix(content, "CUSTOM_SETTING=x\nPOSTGRES_PORT=15432\n") {
|
|
t.Fatalf("expected existing lines to stay in place, got %q", content)
|
|
}
|
|
// The legacy endpoint is dropped: dev runs the object store only.
|
|
want := maps.Clone(composeAppEnv)
|
|
want["CUSTOM_SETTING"] = "x"
|
|
if got := composeEnvFile(t, content); !maps.Equal(got, want) {
|
|
t.Fatalf("expected %v, got %v", want, got)
|
|
}
|
|
if got := composeCalls(t, bin, "docker"); len(got) != 6 || got[0] != "port ods-proj-relational_db-1 5432" {
|
|
t.Fatalf("expected port queries for project ods-proj, got %q", got)
|
|
}
|
|
}
|
|
|
|
func TestRunEnv_dryRunPrintsWithoutWriting(t *testing.T) {
|
|
composeEnvDocker(t)
|
|
root := composeRepo(t)
|
|
|
|
var err error
|
|
stdout := composeCapture(t, &os.Stdout, func() { err = runEnv(true) })
|
|
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got := composeEnvFile(t, stdout); !maps.Equal(got, composeAppEnv) {
|
|
t.Fatalf("expected %v, got %v", composeAppEnv, got)
|
|
}
|
|
if _, statErr := os.Stat(filepath.Join(root, ".vscode")); !os.IsNotExist(statErr) {
|
|
t.Fatalf("expected no .vscode directory, got %v", statErr)
|
|
}
|
|
}
|
|
|
|
func TestRunEnv_errors(t *testing.T) {
|
|
t.Run("missing selected database leaves dotenv unchanged", func(t *testing.T) {
|
|
bin := composeFakeBin(t)
|
|
composeFakeTool(t, bin, "docker", `case "$2" in *-relational_db-1) exit 1 ;; *) echo "0.0.0.0:1$3" ;; esac`)
|
|
root := composeRepo(t)
|
|
envPath := filepath.Join(root, ".vscode", ".env")
|
|
initial := "POSTGRES_PORT=5432\nS3_LEGACY_ENDPOINT_URL=http://localhost:9005\nSECRET=keep\n"
|
|
writeFile(t, envPath, initial)
|
|
if err := runEnv(false); err == nil || !strings.Contains(err.Error(), "PostgreSQL") {
|
|
t.Fatalf("expected missing selected PostgreSQL error, got %v", err)
|
|
}
|
|
if got := composeReadFile(t, envPath); got != initial {
|
|
t.Fatalf("dotenv changed without selected database: %q", got)
|
|
}
|
|
})
|
|
|
|
t.Run("env file is a directory", func(t *testing.T) {
|
|
composeEnvDocker(t)
|
|
root := composeRepo(t)
|
|
envPath := filepath.Join(root, ".vscode", ".env")
|
|
if err := os.MkdirAll(envPath, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
err := runEnv(false)
|
|
want := "Failed to update " + envPath + ": read " + envPath + ": "
|
|
if err == nil || !strings.HasPrefix(err.Error(), want) {
|
|
t.Fatalf("expected error starting with %q, got %v", want, err)
|
|
}
|
|
})
|
|
|
|
t.Run("outside a git repo", func(t *testing.T) {
|
|
composeEnvDocker(t)
|
|
composeRepo(t)
|
|
composeNoRepo(t)
|
|
err := runEnv(false)
|
|
if err == nil || !strings.Contains(err.Error(), "not inside a git worktree") {
|
|
t.Fatalf("expected a git root error, got %v", err)
|
|
}
|
|
})
|
|
}
|