amadan

carlos / platform Public

Merge pull request #71 from carlosframework/worktree-issues-69-70-db-path

registry+cli: db_path rewrites reach running edges; relative -db refused on exec routes
Paul Campbell 934414c88b815295a7514b62a2de357d12b047b1 parent 5695e88parent 8efdf65
5 files changed, +568 −7
  • cli.go +26 −1
  • docs/superpowers/plans/2026-08-06-issues-69-70-db-path.md +380 −0
  • e2e_test.go +132 −0
  • internal/registry/registry.go +10 −6
  • internal/registry/registry_test.go +20 −0
diff --git a/cli.go b/cli.go
index e49017e48..aeca97f16 100644
--- a/cli.go
+++ b/cli.go
@@ -145,6 +145,18 @@ func runAdd(args []string) {
if *backing == registry.BackingUnit && *hibernate && *dbPath == "" {
log.Fatalf("add %s: -backing unit -hibernate requires -db <path> — the box-wide litestream unit can't find a unit tenant's database on its own (see ops.FleetDBs)", host)
}
+ // An agent-exec child gets no STATE_DIRECTORY — the activation
+ // contract's "relative -db resolves inside $STATE_DIRECTORY" convention
+ // is systemd's half, real only under -backing unit. On the exec path the
+ // activator consumes the stored path verbatim (Restore, Seed, Start's
+ // --db flag, the replicator), so a relative path resolves against a cwd
+ // nobody chose and every wake dies as an edge 503 discoverable only in
+ // the journal (issue #70). Refuse it here, at the only door that can
+ // introduce one — every other mint path builds absolute paths.
+ if *backing != registry.BackingUnit && *dbPath != "" && !filepath.IsAbs(*dbPath) {
+ log.Fatalf("add %s: -db %q is relative, and agent-exec children get no STATE_DIRECTORY to resolve it in — pass an absolute path (the default here is %s), or use -backing unit, whose StateDirectory= makes relative paths real",
+ host, *dbPath, provision.DefaultDBPath(dataDir(), host))
+ }
account := resolveAccount(*reg, *acct, *acctID)
// A `daemon` route may not hibernate: the capability means the platform
// owns the process (enable, start, restart-on-adopt), and hibernation is
@@ -170,6 +182,15 @@ func runAdd(args []string) {
// The account is stamped on the row: it is what adopt and the edge build
// this app's bucket keys from. Reads still fall back to the legacy
// unqualified keys, so an unmigrated app added here keeps working.
+ //
+ // An upsert over an existing host is a rewrite, not a creation, and the
+ // verb the operator reads back must say which one happened (issue #69):
+ // "added" printed over a rewrite is how vitogo's operator was left
+ // believing a fresh row existed while a running edge kept the old one.
+ existing, err := r.Get(host)
+ if err != nil {
+ log.Fatal(err)
+ }
if err := r.Upsert(registry.Route{
Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
App: *app, Account: account,
@@ -178,7 +199,11 @@ func runAdd(args []string) {
}); err != nil {
log.Fatal(err)
}
- fmt.Printf("%s %s\n", colorFor(os.Stdout).ok("added"), host)
+ if existing != nil {
+ fmt.Printf("%s %s (existing route)\n", colorFor(os.Stdout).ok("updated"), host)
+ } else {
+ fmt.Printf("%s %s\n", colorFor(os.Stdout).ok("added"), host)
+ }
}
func runRemove(args []string) {
diff --git a/docs/superpowers/plans/2026-08-06-issues-69-70-db-path.md b/docs/superpowers/plans/2026-08-06-issues-69-70-db-path.md
new file mode 100644
index 000000000..17708b6b5
--- /dev/null
+++ b/docs/superpowers/plans/2026-08-06-issues-69-70-db-path.md
@@ -0,0 +1,380 @@
+# Issues #69 + #70: db-path staleness and relative-path failure — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Fix the two defects found in vitogo's first tito deploy (2026-08-06): a re-`add` that changes only `-db` never reaches a running edge (#69), and a relative `-db` on an agent-exec route fails every wake as edge 503s (#70).
+
+**Architecture:** Both root causes are now diagnosed and are NOT what the issue titles guess. (#69) `registry.Upsert` **does** overwrite `db_path` in the DB — the defect is `Registry.Fingerprint()` (the edge's cheap change poll), which sums weighted lengths of `status/socket/addr/version/hibernate/channel/backing/tunnel` but not `db_path`, so a db-path-only rewrite never triggers a route reload and the activator keeps waking the host with the stale cached path. (#70) exec children have no `STATE_DIRECTORY` and the activator/child both consume `rt.DBPath` verbatim (Restore/Seed/Start `--db`/Replicate), so a relative path resolves against an unwritable cwd; every mint path except `carlos add -db` already produces absolute paths, so the fix is a loud add-time refusal at the one door that can introduce the misconfiguration (issue #70's suggested fix 2). We also make `add` say `updated` vs `added` so an upsert-over-existing is visible to the operator (#69's operator-honesty half).
+
+**Tech Stack:** Go, modernc sqlite registry, stdlib flag CLI, e2e tests that build and shell out to the carlos binary.
+
+## Global Constraints
+
+- `go build ./...`, `go vet ./...`, `go test ./... -count=1` all clean (all 18 packages) before any commit lands (CLAUDE.md).
+- `gofmt -l` clean for files touched.
+- Sandbox: go commands need `GOCACHE=/tmp/claude-1001/gocache GOFLAGS=-mod=mod GOPROXY=off` and may need the sandbox disabled ("read-only file system" is the tell).
+- Wire-shape changes are additive: nothing here changes storage; existing registry rows (all absolute paths in production; vitogo's bad row was already removed) must keep parsing.
+- Any e2e test that shells out to the carlos binary must build env through `carlosTestEnv(t)` and set `cmd.Dir` to a scratch dir (the binary walks cwd ancestors for `.carlos/config`).
+- Working branch: `worktree-issues-69-70-db-path` (already created). Never push main; open a PR at the end.
+- Delegation (CLAUDE.md): registry/CLI door is production-touching — Opus reviews the finished branch before the PR opens.
+
+---
+
+### Task 1: Fingerprint sees db_path rewrites (#69 core)
+
+**Files:**
+- Modify: `internal/registry/registry.go` (Fingerprint, ~line 1009)
+- Test: `internal/registry/registry_test.go` (extend the existing fingerprint test that ends with the tunnel-binding check, ~line 190)
+
+**Interfaces:**
+- Consumes: existing `Registry.Fingerprint() (string, error)`, `Registry.Get/Upsert`.
+- Produces: no signature changes — only the fingerprint SQL gains a `LENGTH(db_path)*19` term. Weight 19 is the next unused prime (3,5,7,11,13 are column weights; 17 is the rowid multiplier).
+
+- [ ] **Step 1: Write the failing test**
+
+In `internal/registry/registry_test.go`, extend the existing fingerprint test (the one containing `t.Fatal("fingerprint blind to a tunnel binding")`) by appending, after that final check:
+
+```go
+ // db_path rewrites must reload the edge too: a re-`carlos add` that only
+ // fixes -db otherwise leaves the running activator waking the host with
+ // the stale cached path forever — the exact vitogo incident (issue #69):
+ // `add` reported success, the row was updated, and wakes still died on
+ // the old path minutes later because this poll never moved.
+ rt, err = reg.Get("c.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ rt.DBPath = "/data/somewhere-else.db"
+ if err := reg.Upsert(*rt); err != nil {
+ t.Fatal(err)
+ }
+ withDBPath, err := reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if withDBPath == withTunnel {
+ t.Fatal("fingerprint blind to a db_path rewrite")
+ }
+```
+
+Note: `rt` and `err` already exist in that scope (the backing-flip check declared them), so this is `rt, err =`, not `:=`. If the route `c.example.com` in that test already carries a `DBPath`, pick a replacement value of a **different length** — the fingerprint is length-weighted by design and a same-length rewrite is deliberately invisible.
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `go test ./internal/registry/ -run 'Fingerprint' -count=1 -v` (with the sandbox go env vars)
+Expected: FAIL with `fingerprint blind to a db_path rewrite`
+
+- [ ] **Step 3: Add the term to Fingerprint**
+
+In `internal/registry/registry.go`, change the SUM expression inside `Fingerprint()`:
+
+```go
+ COALESCE(SUM(rowid*17 + LENGTH(status)*3 + LENGTH(socket) + LENGTH(addr)
+ + LENGTH(version)*5 + hibernate*7 + LENGTH(channel)
+ + LENGTH(backing)*11 + LENGTH(tunnel)*13 + LENGTH(db_path)*19),0)
+```
+
+And extend the function's doc comment: after the sentence about weights, add one sentence — `db_path is in the sum (issue #69): a re-add that only re-points a route's database must reach a running edge, or the activator keeps waking the host on the stale cached path.`
+
+- [ ] **Step 4: Run the test to verify it passes**
+
+Run: `go test ./internal/registry/ -count=1` (full package — the fingerprint change can break other fingerprint assertions if any pin exact values; fix only by updating pinned strings, never by weakening assertions)
+Expected: PASS
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add internal/registry/registry.go internal/registry/registry_test.go
+git commit -m "registry: fingerprint sees db_path rewrites (#69)"
+```
+
+### Task 2: `carlos add` says updated vs added (#69 operator honesty)
+
+**Files:**
+- Modify: `cli.go` (`runAdd`, the tail: the `registry.Open` → `Upsert` → `fmt.Printf("added %s\n", host)` block, ~lines 148–161)
+- Test: `e2e_test.go` (new test function, following the shape of the `-account-id` refusal test at ~line 1370)
+
+**Interfaces:**
+- Consumes: `registry.Registry.Get(host string) (*Route, error)` (existing), `registry.Registry.Upsert(Route) error` (existing).
+- Produces: `carlos add` prints `updated <host> (existing route)` when the host row already existed, `added <host>` otherwise. No flag changes.
+
+- [ ] **Step 1: Write the failing test**
+
+Add to `e2e_test.go`:
+
+```go
+// carlos add on an existing host is an update and must say so — the vitogo
+// incident (issue #69) started with `added vito.platform.tito.io` printed
+// over what was actually a rewrite of an existing row.
+func TestAddOnExistingRouteSaysUpdated(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", err, out)
+ }
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ cmd := exec.Command(carlosBin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = tmp
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ "-db", "/data/first.db", "vito.example.com")
+ if err != nil {
+ t.Fatalf("first add: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "added vito.example.com") {
+ t.Errorf("first add output = %q, want %q", out, "added vito.example.com")
+ }
+
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-db", "/data/second.db", "vito.example.com")
+ if err != nil {
+ t.Fatalf("re-add: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "updated vito.example.com") {
+ t.Errorf("re-add output = %q, want it to say updated, not added", out)
+ }
+
+ // And the rewrite must actually land — the row carries the new path.
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get("vito.example.com")
+ if err != nil || rt == nil {
+ t.Fatalf("get route: %v (rt=%v)", err, rt)
+ }
+ if rt.DBPath != "/data/second.db" {
+ t.Errorf("db_path = %q, want /data/second.db", rt.DBPath)
+ }
+}
+```
+
+(`registry` is already imported in `e2e_test.go`; `os`, `strings`, `filepath`, `exec` too — verify and add any missing import.)
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `go test . -run TestAddOnExistingRouteSaysUpdated -count=1 -v`
+Expected: FAIL on the `updated` assertion (output still says `added`)
+
+- [ ] **Step 3: Implement**
+
+In `cli.go` `runAdd`, after `r, err := registry.Open(*reg)` succeeds and before the `Upsert` call, look up the existing row; after the `Upsert`, report accordingly. Replace the tail:
+
+```go
+ // An upsert over an existing host is a rewrite, not a creation, and the
+ // verb the operator reads back must say which one happened (issue #69):
+ // "added" printed over a rewrite is how vitogo's operator was left
+ // believing a fresh row existed while a running edge kept the old one.
+ existing, err := r.Get(host)
+ if err != nil {
+ log.Fatal(err)
+ }
+ if err := r.Upsert(registry.Route{
+ Host: host, Slug: *slug, Kind: *kind, Socket: *socket, Addr: *addr,
+ App: *app, Account: account,
+ Channel: *channel, Hibernate: *hibernate, StatusNamed: *statusNamed, Backing: *backing,
+ DBPath: *dbPath, Status: "ready",
+ }); err != nil {
+ log.Fatal(err)
+ }
+ if existing != nil {
+ fmt.Printf("updated %s (existing route)\n", host)
+ } else {
+ fmt.Printf("added %s\n", host)
+ }
+```
+
+(The comment above the Upsert about account stamping stays where it is.)
+
+- [ ] **Step 4: Run the test to verify it passes**
+
+Run: `go test . -run TestAddOnExistingRouteSaysUpdated -count=1 -v`
+Expected: PASS
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add cli.go e2e_test.go
+git commit -m "cli: carlos add says updated when the route already existed (#69)"
+```
+
+### Task 3: refuse a relative -db on agent-exec routes (#70)
+
+**Files:**
+- Modify: `cli.go` (`runAdd`, immediately after the `-backing unit -hibernate requires -db` refusal, ~line 128)
+- Test: `e2e_test.go` (new test function, same scaffold as Task 2)
+
+**Interfaces:**
+- Consumes: `filepath.IsAbs` (`path/filepath` is already imported in `cli.go` — verify), `registry.BackingUnit` (existing).
+- Produces: `carlos add` with `-backing ""` (exec child) and a relative non-empty `-db` exits non-zero with a message naming the cause (no `STATE_DIRECTORY` for exec children), the requirement (absolute path), and the convention (`provision.DefaultDBPath` result). `-backing unit` keeps accepting relative paths — systemd's `StateDirectory=carlos-app/%i` is what makes the rastrillo activation-contract convention real there.
+
+- [ ] **Step 1: Write the failing test**
+
+Add to `e2e_test.go`:
+
+```go
+// A relative -db on an agent-exec route must be refused at add time. The
+// activation contract's "relative -db resolves inside $STATE_DIRECTORY"
+// convention is real for unit tenants (systemd sets it) but agent-exec
+// children are spawned without it and the activator consumes the path
+// verbatim (Restore, Seed, Start --db, Replicate) — so a relative path
+// resolves against an unwritable cwd and every wake dies as an edge 503,
+// discoverable only by correlating 503s with child stderr in the journal
+// (issue #70, vitogo's first deploy).
+func TestAddRefusesRelativeDBOnExecRoutes(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", err, out)
+ }
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ cmd := exec.Command(carlosBin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = tmp
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ // The vitogo add, verbatim shape: exec backing (the default), relative -db.
+ out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ "-kind", "instance", "-db", "vito.db", "-hibernate",
+ "-channel", "canary/vitogo", "vito.example.com")
+ if err == nil {
+ t.Fatalf("add with relative -db on an exec route: want non-zero exit, got 0\n%s", out)
+ }
+ // The refusal must carry the diagnosis and the way out — not just "no".
+ if !strings.Contains(out, "STATE_DIRECTORY") || !strings.Contains(out, "absolute") {
+ t.Errorf("refusal = %q, want it to explain STATE_DIRECTORY and require an absolute path", out)
+ }
+ // And it must refuse BEFORE writing: no half-registered route.
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get("vito.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if rt != nil {
+ t.Fatalf("refused add still wrote a route: %+v", rt)
+ }
+
+ // A unit tenant keeps the contract: StateDirectory= is real there, a
+ // relative -db is the documented convention, and this guard must not
+ // take it away.
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-kind", "instance", "-backing", "unit", "-db", "vito.db",
+ "unit.example.com")
+ if err != nil {
+ t.Fatalf("add -backing unit with relative -db: %v\n%s", err, out)
+ }
+
+ // An absolute -db on an exec route is untouched by the guard.
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-kind", "instance", "-db", "/data/vito.example.com.db",
+ "vito.example.com")
+ if err != nil {
+ t.Fatalf("add with absolute -db: %v\n%s", err, out)
+ }
+}
+```
+
+- [ ] **Step 2: Run the test to verify it fails**
+
+Run: `go test . -run TestAddRefusesRelativeDBOnExecRoutes -count=1 -v`
+Expected: FAIL at the first assertion (the add exits 0 today)
+
+- [ ] **Step 3: Implement the guard**
+
+In `cli.go` `runAdd`, directly after the `-backing unit -hibernate requires -db` refusal block (so it runs after the `-db` defaulting, before `resolveAccount`):
+
+```go
+ // An agent-exec child gets no STATE_DIRECTORY — the activation
+ // contract's "relative -db resolves inside $STATE_DIRECTORY" convention
+ // is systemd's half, real only under -backing unit. On the exec path the
+ // activator consumes the stored path verbatim (Restore, Seed, Start's
+ // --db flag, the replicator), so a relative path resolves against a cwd
+ // nobody chose and every wake dies as an edge 503 discoverable only in
+ // the journal (issue #70). Refuse it here, at the only door that can
+ // introduce one — every other mint path builds absolute paths.
+ if *backing != registry.BackingUnit && *dbPath != "" && !filepath.IsAbs(*dbPath) {
+ log.Fatalf("add %s: -db %q is relative, and agent-exec children get no STATE_DIRECTORY to resolve it in — pass an absolute path (the default here is %s), or use -backing unit, whose StateDirectory= makes relative paths real",
+ host, *dbPath, provision.DefaultDBPath(dataDir(), host))
+ }
+```
+
+Verify `path/filepath` and the `provision` package are in `cli.go`'s imports (both should be — `runAdd` already calls `filepath.Join` and `provision.DefaultDBPath`); add if missing.
+
+- [ ] **Step 4: Run the test to verify it passes**
+
+Run: `go test . -run TestAddRefusesRelativeDBOnExecRoutes -count=1 -v`
+Expected: PASS
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add cli.go e2e_test.go
+git commit -m "cli: refuse a relative -db on agent-exec routes (#70)"
+```
+
+### Task 4: full verification, Opus review, PR
+
+**Files:**
+- No new code; fixes from review land as amendments to the files above.
+
+- [ ] **Step 1: Full suite**
+
+Run (sandbox env vars, sandbox disabled if the toolchain complains):
+```bash
+go build ./... && go vet ./... && go test ./... -count=1
+gofmt -l internal/registry/registry.go internal/registry/registry_test.go cli.go e2e_test.go
+```
+Expected: build/vet clean, all packages pass, gofmt prints nothing.
+
+- [ ] **Step 2: Opus review (CLAUDE.md: production-touching work gets an Opus reviewer)**
+
+Dispatch a review subagent on **Opus** over `git diff main...HEAD`, briefed with both issue bodies and the two root-cause diagnoses (fingerprint blindness; exec-verbatim path consumption). Questions it must answer: does the fingerprint term break any pinned-value test; does the updated/added lookup introduce a TOCTOU worth caring about (two concurrent adds — acceptable: the message is advisory); does the relative-db guard block any legitimate existing workflow (unit tenants must keep relative). Apply fixes, re-run the full suite, amend commits or add a fix commit.
+
+- [ ] **Step 3: Push and open the PR**
+
+```bash
+git push -u origin worktree-issues-69-70-db-path
+gh pr create --repo carlosframework/platform --draft \
+ --title "registry+cli: db_path reaches running edges; relative -db refused on exec routes" \
+ --body "Fixes #69, fixes #70. ..."
+```
+
+PR body must state both root-cause diagnoses (they differ from the issue titles' guesses), what changed, and the deferred follow-up: materializing a real per-host `STATE_DIRECTORY` for exec children (issue #70's suggested fix 1) is a design decision — a new on-disk convention with migration implications — left open on the issue, not taken overnight.
+
+Expected: draft PR open, linked to both issues.
+
+---
+
+## Self-review notes
+
+- **#69 coverage:** the issue's observed failure (success message + stale behavior) is covered twice — fingerprint (behavioral root cause, Task 1) and updated/added honesty (message half, Task 2). The issue's literal guess ("stored db path silently kept its original value") is disproven by `Upsert`'s `db_path=excluded.db_path`; Task 2's test pins the row rewrite to guard it.
+- **#70 coverage:** suggested fix 2, chosen over 1/3 deliberately: resolving relative paths platform-side (fix 3) invents a per-host state-dir convention overnight and threads it through Restore/Seed/Start/Replicate **and** `ops.FleetDBs`' path matching (`filepath.Clean(r.DBPath)`) **and** `caps_pass` stamping — fleet-wide blast radius for a case the door can simply refuse. Fix 1 (real STATE_DIRECTORY) is the same convention decision; deferred explicitly on the issue.
+- **Sticky-column caveat:** `Upsert` deliberately keeps `environment/caps/tunnel/provisioned/seed` when incoming is zero; none of these is a `carlos add` flag, so "updates all supplied fields" is accurate as printed.
+- **Type consistency:** Tasks 2 and 3 both edit `runAdd`; Task 3's guard sits above Task 2's `existing` lookup and they don't interact. Both e2e tests use the local-build scaffold (each e2e test builds its own binary — the file's existing convention) plus `carlosTestEnv` + `cmd.Dir` per CLAUDE.md.
diff --git a/e2e_test.go b/e2e_test.go
index bbde67b4d..97539e2c4 100644
--- a/e2e_test.go
+++ b/e2e_test.go
@@ -1353,6 +1353,80 @@ func TestAddUnitBackingRequiresDBForHibernate(t *testing.T) {
}
}
+// A relative -db on an agent-exec route must be refused at add time. The
+// activation contract's "relative -db resolves inside $STATE_DIRECTORY"
+// convention is real for unit tenants (systemd sets it) but agent-exec
+// children are spawned without it and the activator consumes the path
+// verbatim (Restore, Seed, Start --db, Replicate) — so a relative path
+// resolves against an unwritable cwd and every wake dies as an edge 503,
+// discoverable only by correlating 503s with child stderr in the journal
+// (issue #70, vitogo's first deploy).
+func TestAddRefusesRelativeDBOnExecRoutes(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", err, out)
+ }
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ cmd := exec.Command(carlosBin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = tmp
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ // The vitogo add, verbatim shape: exec backing (the default), relative -db.
+ out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ "-kind", "instance", "-db", "vito.db", "-hibernate",
+ "-channel", "canary/vitogo", "vito.example.com")
+ if err == nil {
+ t.Fatalf("add with relative -db on an exec route: want non-zero exit, got 0\n%s", out)
+ }
+ // The refusal must carry the diagnosis and the way out — not just "no".
+ if !strings.Contains(out, "STATE_DIRECTORY") || !strings.Contains(out, "absolute") {
+ t.Errorf("refusal = %q, want it to explain STATE_DIRECTORY and require an absolute path", out)
+ }
+ // And it must refuse BEFORE writing: no half-registered route.
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get("vito.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if rt != nil {
+ t.Fatalf("refused add still wrote a route: %+v", rt)
+ }
+
+ // A unit tenant keeps the contract: StateDirectory= is real there, a
+ // relative -db is the documented convention, and this guard must not
+ // take it away.
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-kind", "instance", "-backing", "unit", "-db", "vito.db",
+ "unit.example.com")
+ if err != nil {
+ t.Fatalf("add -backing unit with relative -db: %v\n%s", err, out)
+ }
+
+ // An absolute -db on an exec route is untouched by the guard.
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-kind", "instance", "-db", "/data/vito.example.com.db",
+ "vito.example.com")
+ if err != nil {
+ t.Fatalf("add with absolute -db: %v\n%s", err, out)
+ }
+}
+
// TestAddAccountIDRefusesAName pins the whole -account-id gate end to end,
// on the verb where getting it wrong is quietest. `carlos add -account-id
// ops` used to succeed: "ops" passes release.ValidAccount (^[a-z]{3,16}$),
@@ -1437,6 +1511,64 @@ func TestAddAccountIDRefusesAName(t *testing.T) {
}
}
+// carlos add on an existing host is an update and must say so — the vitogo
+// incident (issue #69) started with `added vito.platform.tito.io` printed
+// over what was actually a rewrite of an existing row.
+func TestAddOnExistingRouteSaysUpdated(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", err, out)
+ }
+ regPath := filepath.Join(tmp, "registry.db")
+
+ run := func(args ...string) (string, error) {
+ cmd := exec.Command(carlosBin, args...)
+ cmd.Env = carlosTestEnv(t)
+ cmd.Dir = tmp
+ out, err := cmd.CombinedOutput()
+ return string(out), err
+ }
+
+ out, err := run("add", "-registry", regPath, "-app", "vitogo",
+ "-db", "/data/first.db", "vito.example.com")
+ if err != nil {
+ t.Fatalf("first add: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "added vito.example.com") {
+ t.Errorf("first add output = %q, want %q", out, "added vito.example.com")
+ }
+
+ out, err = run("add", "-registry", regPath, "-app", "vitogo",
+ "-db", "/data/second.db", "vito.example.com")
+ if err != nil {
+ t.Fatalf("re-add: %v\n%s", err, out)
+ }
+ if !strings.Contains(out, "updated vito.example.com") {
+ t.Errorf("re-add output = %q, want it to say updated, not added", out)
+ }
+
+ // And the rewrite must actually land — the row carries the new path.
+ reg, err := registry.Open(regPath)
+ if err != nil {
+ t.Fatalf("open registry: %v", err)
+ }
+ defer reg.Close()
+ rt, err := reg.Get("vito.example.com")
+ if err != nil || rt == nil {
+ t.Fatalf("get route: %v (rt=%v)", err, rt)
+ }
+ if rt.DBPath != "/data/second.db" {
+ t.Errorf("db_path = %q, want /data/second.db", rt.DBPath)
+ }
+}
+
// An app still on the pre-accounts keys can be moved INTO ops. Until `-from
// legacy` existed there was no way to say it: a bare move into ops is refused
// (ops is also the default source, and the verb will not guess a direction),
diff --git a/internal/registry/registry.go b/internal/registry/registry.go
index fc5bf7520..8fee34533 100644
--- a/internal/registry/registry.go
+++ b/internal/registry/registry.go
@@ -1097,7 +1097,7 @@ WHERE kind = 'instance' AND next_due > 0 AND next_due <= ? ORDER BY host`, now)
// Fingerprint summarizes the route table over a fresh connection, for the
// edge's cheap change poll. It is content-accurate over the reload-relevant
// columns (status, socket, addr, version, hibernate, channel, backing,
-// tunnel): it hashes the actual values, not a length-weighted aggregate, so
+// tunnel, db_path): it hashes the actual values, not a length-weighted aggregate, so
// two DIFFERENT values of the SAME length (channel canary/1 -> canary/2, a
// git short-sha version bump) change the string exactly like any other
// write. `grants` stays deliberately excluded (registry.go:93's rule: grants
@@ -1113,6 +1113,10 @@ WHERE kind = 'instance' AND next_due > 0 AND next_due <= ? ORDER BY host`, now)
// construction — the old length-weighted aggregate could not see a version
// bump land, so the header would serve stale until an unrelated column
// happened to change too.
+//
+// db_path is hashed too (issue #69): a re-add that only re-points a route's
+// database must reach a running edge, or the activator keeps waking the host
+// on the stale cached path.
func (r *Registry) Fingerprint() (string, error) {
db, err := sql.Open("sqlite", r.path)
if err != nil {
@@ -1123,7 +1127,7 @@ func (r *Registry) Fingerprint() (string, error) {
return "", err
}
rows, err := db.Query(`
-SELECT rowid, status, socket, addr, version, hibernate, channel, backing, tunnel
+SELECT rowid, status, socket, addr, version, hibernate, channel, backing, tunnel, db_path
FROM routes ORDER BY rowid`)
if err != nil {
return "", err
@@ -1133,12 +1137,12 @@ FROM routes ORDER BY rowid`)
n := 0
for rows.Next() {
var rowid, hibernate int64
- var status, socket, addr, version, channel, backing, tunnel string
- if err := rows.Scan(&rowid, &status, &socket, &addr, &version, &hibernate, &channel, &backing, &tunnel); err != nil {
+ var status, socket, addr, version, channel, backing, tunnel, dbPath string
+ if err := rows.Scan(&rowid, &status, &socket, &addr, &version, &hibernate, &channel, &backing, &tunnel, &dbPath); err != nil {
return "", err
}
- fmt.Fprintf(h, "%d\x1f%s\x1f%s\x1f%s\x1f%s\x1f%d\x1f%s\x1f%s\x1f%s\x1e",
- rowid, status, socket, addr, version, hibernate, channel, backing, tunnel)
+ fmt.Fprintf(h, "%d\x1f%s\x1f%s\x1f%s\x1f%s\x1f%d\x1f%s\x1f%s\x1f%s\x1f%s\x1e",
+ rowid, status, socket, addr, version, hibernate, channel, backing, tunnel, dbPath)
n++
}
if err := rows.Err(); err != nil {
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go
index dc6a24ff2..b571d12de 100644
--- a/internal/registry/registry_test.go
+++ b/internal/registry/registry_test.go
@@ -190,6 +190,26 @@ func TestFingerprintChangesOnWrite(t *testing.T) {
if withTunnel == withBacking {
t.Fatal("fingerprint blind to a tunnel binding")
}
+ // db_path rewrites must reload the edge too: a re-`carlos add` that only
+ // fixes -db otherwise leaves the running activator waking the host with
+ // the stale cached path forever — the exact vitogo incident (issue #69):
+ // `add` reported success, the row was updated, and wakes still died on
+ // the old path minutes later because this poll never moved.
+ rt, err = reg.Get("c.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ rt.DBPath = "/data/somewhere-else.db"
+ if err := reg.Upsert(*rt); err != nil {
+ t.Fatal(err)
+ }
+ withDBPath, err := reg.Fingerprint()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if withDBPath == withTunnel {
+ t.Fatal("fingerprint blind to a db_path rewrite")
+ }
}
func TestSetVersionsAndHibernateAreNarrow(t *testing.T) {