[ac-pplx]

Summary

Adds two nullable columns to sandboxes for the node's attribution of a
sandbox's last stop. #1520 writes and reads them.

• stop_cause text: why the sandbox stopped when no one asked it to
  (pod_terminated or vm_exited today). It is NULL for a requested stop.
  The labels belong to the writer's enum, so there is no CHECK.
• stop_cause_at timestamptz: when the node stopped the sandbox, by the
  node's clock. It is set for a requested stop too. Both columns NULL means no
  attribution is recorded.

lifecycle_changed_at can't stand in for stop_cause_at. It is the control
plane's time of the convergence, one publication after the stop, and later
lifecycle changes, an evacuation included, move it.

A BEFORE UPDATE trigger, sandboxes_clear_stop_cause, clears both columns
when the lifecycle changes to started or paused, meaning the sandbox runs
again. Every other move keeps them, including a stopped sandbox's evacuation
(stopped → restoring → stopped) and a delete. It follows the three
sandboxes_clear_* triggers.

Nothing reads or writes the columns until #1520 lands. This revision changes
only comments: the column comment describes the node's attribution, and the
down migration names internal-api.

Validation

• scripts/check-postgres-migrations-isolated.sh: "Postgres migrations changed
  without runtime app changes; migration isolation check passed."
• scripts/check-migration-versions.sh: passes.
• scripts/check-postgres-migrations-squawk.sh: 0 issues.
• The migration applies through the nextest Postgres fixture. api: record why a sandbox stopped when its pod or VM died #1520's
  store tests exercise the trigger, against this file:
  • an attribution survives a drain's evacuation;
  • reaching started clears it.
• With the previous trigger, which cleared on any move out of stopped, the
  evacuation case fails (left: None, right: Some("pod_terminated")).
• Kind: the Kubernetes and Direct stacks of ui: show why a sandbox stopped unexpectedly #1521 ran with this migration
  applied by space-db-migrate.

Severity and risk

Severity: does not apply on its own; it enables #1520.

Risk: ALTER TABLE ... ADD COLUMN with no default doesn't rewrite the table,
but it takes a brief ACCESS EXCLUSIVE lock on the hot sandboxes table.

• The file sets SET LOCAL lock_timeout = '1s' and
  statement_timeout = '5s', so a lost lock race fails fast with 55P03,
  and the migrator retries it.
• The trigger costs one lifecycle comparison per row update, like the three
  sandboxes_clear_* triggers.

Rollout

Land and deploy this before #1520. #1520's API selects the new columns in its
shared sandbox column list, on every sandbox read, so it fails against a
database without them.

An API without #1520 never reads the columns, and nothing writes them, so this
change does nothing on its own.

Rollback: leave the columns in place; they are inert without #1520. Run the
down migration only once neither #1520's API nor its internal-api is
deployed:

• the API selects the columns on every sandbox read;
• internal-api writes them inside the node-publish transaction, so dropping
  them fails every publication from a node that holds a stopped sandbox.

The migrator doesn't run down migrations on deploy, so a rollback is a
separate, deliberate step.

Stack

#1520 (api: record why a sandbox stopped when its pod or VM died) is stacked
on this and merges after it, and #1521 (ui) on #1520.

AI assistance: Claude Code wrote the migration and this description; a human
reviews them before this pull request opens.

[coderabbitai]

Automatic approval withheld

Automatic approval is disabled by the configured approval policy for this pull request. Review comments and requests for changes continue normally.

Commit: d5c1e372398d.

No actionable comments were generated in the recent review.

Walkthrough

The migration adds nullable stop_cause and stop_cause_at columns to public.sandboxes. A trigger clears both fields when the lifecycle changes to started or paused, after trimming and lowercasing the lifecycle value. The down migration removes the trigger, its function, and both columns. The migrations set local lock and statement timeouts.

Merge Risk: High · up to d5c1e

The migration changes which stop attribution a sandbox retains after lifecycle updates. The inspected trigger matches the intended transitions, but this lifecycle-write change warrants validation before merge.

Architecture risk: Low · up to d5c1e

The change affects 1 system.

Changed systems: db

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — db (service) was modified; 2 changed files map to changed impact.

Before / after behavior

• observed — Modified behavior in db/postgres/20261105163812_sandbox_stop_cause.down.sql: The migration sets local lock and statement timeouts, then removes the stop-cause trigger, function, and columns if present. Comments describe the deployed-service usage condition for running these destructive schema changes.
• observed — Modified behavior in db/postgres/20261105163812_sandbox_stop_cause.up.sql: The migration sets local lock and statement timeouts to 1 and 5 seconds, respectively, and adds nullable stop_cause and stop_cause_at columns to public.sandboxes without defaults, backfill, checks, or indexes. The added comments describe the columns’ attribution and timestamp semantics.
• observed — Modified behavior in db/postgres/20261105163812_sandbox_stop_cause.up.sql: Adds public.sandboxes_clear_stop_cause() and a BEFORE UPDATE row trigger. When lifecycle changes to started or paused after trimming and lowercasing, the function clears both attribution columns; otherwise it leaves them unchanged.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title starts with the lowercase area prefix "db:", uses the lowercase summary "add", and accurately describes the added sandbox stop-cause columns.
Description Follows The Pr Template		The description contains all four required items: Summary, Validation, Severity and risk, and AI assistance. Each has substantive content.
Bug Fixes Name A Regression Test		The PR adds database migrations for new stop attribution fields. It does not fix an application defect under crates/, shim/, or ui/.
Weakened Safeguards Are Justified		The diff adds two migration files. It does not weaken an existing test, assertion, lint, CI check, or repository guard.
Public-Safe Content		The diff, title, and description contain no content that matches the listed failure conditions.

Recent review info

Automatic approval policy

Commit: d5c1e372398d.

Setting	Effective value	Source
Automatic approvals	Disabled	Override 4
Merge risk	Low	Override 2
Security risk	Not set	Global configuration

Matching overrides (numbered as in the resolved configuration):

• Override 2 — any author; paths: **, !ui/**; merge risk: Low.
• Override 4 — any author; paths: .coderabbit.yaml, .coderabbit.yml, coderabbit.yaml, coderabbit.yml, .coderabbit.config.ts, **/AGENTS.md, **/CLAUDE.md, .agents/**, .claude/**, .claude-plugin/**, CONTRIBUTING.md, docs/versioning.md, docs/rust/space-conventions.md, docs/rust/space-review-lessons.md, .github/**, ci/**, !ci/scripts/setup-nextest-*.sh, scripts/source_guards/**, scripts/check*, clippy.toml, deny.toml, .gitleaks.toml, .pre-commit-config.yaml, lefthook.yaml, rust-toolchain.toml, .cargo/**, supply-chain/**, pyproject.toml, uv.lock, **/package.json, **/package-lock.json, **/go.mod, **/go.sum, **/requirements*.txt, **/requirements*.in, proto/**, db/**, crates/l40_api/api/src/auth.rs, crates/l40_api/api/src/auth/**, crates/l40_api/api/src/middleware/auth*, crates/l40_api/api/src/middleware/auth/**, crates/l40_api/api/src/routes/authz.rs, crates/l10_foundation/bearer/**, crates/l10_foundation/rustls/**, crates/l10_foundation/guest/**, crates/l10_foundation/pii/**, shim/**, docker/**, !docker/Dockerfile.rust-build-kind, templates/**, infra/space/aws/**, infra/space/cluster/**, infra/space/identity/**, infra/space/observability/**, infra/space/packer/**, infra/space/kind/charts/**, LICENSE, SECURITY.md; automatic approvals: disabled.

Any matching disabled override withholds approval, regardless of risk. Otherwise, the strictest explicitly set limit wins for each score. Limits omitted by all matching overrides inherit the global configuration.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: perplexityai/SPACE
• Branch: main
• File: .coderabbit.yaml
• Commit: 7cbfbb7
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: a4ae8d49-9176-4141-ad8a-37aa738280d3

Commits

Reviewing files that changed from the base of the PR and between 7cbfbb7 and d5c1e37.

Files selected for processing: 2

• db/postgres/20261105163812_sandbox_stop_cause.down.sql
• db/postgres/20261105163812_sandbox_stop_cause.up.sql

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 3 remain after this review.

[github-actions]

Thanks for opening this pull request! A SPACE maintainer will take a look.

CONTRIBUTING.md explains how we review outside contributions.

[ac-pplx]

Closing: we decided not to add schema columns to record a stop's cause. The value doesn't justify a new column, a trigger on sandboxes, and the ingest rule. The branch stays in place in case we revisit.

[shehan-pplx]

Reviewed by Shehan's Review Bot

[shehan-pplx]

[Consider] Enforce the pairing the comment above defines: a stop_cause always comes with its stop_cause_at (a requested stop is (NULL, time), no attribution is (NULL, NULL), and (label, NULL) is never valid).

Why: #1520's reader (stop_cause_response) needs both (sandbox.stop_cause_at?), so a row with a label and no time would silently read as a plain stop. The ingest only writes rows where stop_cause_at IS NOT NULL today, but the schema is the one place that holds this for every writer, including manual fixes and future verbs. Skipping a CHECK on the labels makes sense (the writer owns the enum); this one is about the shape of the pair, not the labels.

Add it to the same ALTER TABLE as NOT VALID, so it takes no extra lock and doesn't scan the table under the 5s statement_timeout. The columns are new and all NULL, so there's nothing to validate. The down migration's DROP COLUMN drops it.

Suggested change

	-- Nullable with no default, backfill, CHECK or index: the labels belong to the
	-- writer's enum, and nothing filters on either column.
	ALTER TABLE public.sandboxes
	ADD COLUMN stop_cause text,
	ADD COLUMN stop_cause_at timestamp with time zone;
	-- Nullable with no default, backfill or index, and no CHECK on the labels:
	-- they belong to the writer's enum, and nothing filters on either column. A
	-- cause always carries its time.
	ALTER TABLE public.sandboxes
	ADD COLUMN stop_cause text,
	ADD COLUMN stop_cause_at timestamp with time zone,
	ADD CONSTRAINT sandboxes_stop_cause_has_time
	CHECK (stop_cause IS NULL OR stop_cause_at IS NOT NULL) NOT VALID;