Skip to content

db: keep updated_at when a sandbox's idle deadline changes - #1616

Open
NancyKwongPerplexity wants to merge 1 commit into
mainfrom
sandbox-expires-at-trigger
Open

NancyKwongPerplexity wants to merge 1 commit into
mainfrom
sandbox-expires-at-trigger

Conversation

@NancyKwongPerplexity

@NancyKwongPerplexity NancyKwongPerplexity commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds next_auto_event_at to the columns the sandboxes_set_updated_at trigger ignores. The migration 20261106045040_sandboxes_updated_at_ignores_idle_deadline replaces only the trigger function, set_sandbox_updated_at.

sandboxes.next_auto_event_at is the idle deadline that getSandbox and listSandboxes report as expires_at. The stacked change #1617 has nodes publish it. It then moves whenever the sandbox is used, as last_access_at and last_command_at do. The trigger already ignores those activity clocks so that use doesn't read as a modification. Without this migration, every published deadline change would advance the public updated_at, about once a minute on an active sandbox.

Validation

  • scripts/check-migration-versions.sh, scripts/check-postgres-migrations-isolated.sh and scripts/check-postgres-migrations-squawk.sh against origin/main: all pass (squawk 0 issues).

  • The behavior is pinned by tests in fix(api): report the node's idle deadline as a sandbox's expires_at #1617, because this PR's isolation guard allows only migration files:

    • writing_the_idle_deadline_leaves_updated_at_alone (sandbox store), which also checks that a real change still bumps updated_at;
    • the updated_at assertion in publish_sandbox_state_records_the_idle_deadline_without_touching_updated_at (internal API).

    Both fail with this migration removed ("a published idle deadline must not bump updated_at") and pass with it.

  • Applied by the Kind bring-up (_sqlx_migrations records 20261106045040, success). With fix(api): report the node's idle deadline as a sandbox's expires_at #1617 deployed on top, updated_at stayed at its :set_timeout value while the published deadline landed.

  • At 574d29d: cargo test for pplx-space-db-migrate (59 passed) and pplx-space-sandbox-store (33 passed), which apply every migration to a fresh database; cargo clippy with -D warnings on both; cargo check --workspace --all-targets --locked; and cargo fmt --check. All pass.

Severity and risk

Severity: does not apply on its own. It prepares #1617.

Risk: the function body is otherwise identical to the baseline's. The migration sets lock_timeout and statement_timeout, and it changes no rows. Rolling back restores the previous function. That is harmless until #1617 runs, after which deadline publications would advance updated_at again.

Rollout

Deploy with or before #1617. Until #1617, nothing writes the column except the existing reset to NULL, so behavior does not change.

Stack

Base of the stack: this PR, then #1617.

AI assistance: Claude Code drafted the code, tests, and this description; the author reviewed them.

`sandboxes.next_auto_event_at` is the idle deadline that `getSandbox`
reports as `expires_at`. Once nodes publish it, it moves whenever the
sandbox is used, as `last_access_at` and `last_command_at` do. The
`sandboxes_set_updated_at` trigger ignores those activity clocks so that
use does not read as a modification. This adds the deadline to the same
list, so publishing it will not advance the public `updated_at`.

The migration only replaces the trigger function; it changes no rows.
The down migration restores the previous function.
@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Explain Change →

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: 574d29d2cee7.

No actionable comments were generated in the recent review.

Walkthrough

The up and down migrations set local lock and statement timeouts and define public.set_sandbox_updated_at(). Both preserve an explicitly changed updated_at and advance it when other compared columns differ. The up migration excludes next_auto_event_at from those comparisons; the down migration does not.

Merge Risk: Moderate · up to 574d2

Deadline-only updates will no longer advance a sandbox’s public timestamp. This is the intended behavior, but it changes a database row-write contract and warrants explicit acceptance before merge.

Architecture risk: Low · up to 574d2

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/20261106045040_sandboxes_updated_at_ignores_idle_deadline.down.sql: Adds transaction-local lock and statement timeouts and replaces public.set_sandbox_updated_at() with trigger logic that preserves explicitly changed timestamps, updates the timestamp for changes outside the four excluded columns, and retains the old timestamp for changes limited to those columns.
  • observed — Modified behavior in db/postgres/20261106045040_sandboxes_updated_at_ignores_idle_deadline.up.sql: Adds local lock and statement timeouts, then defines a trigger function that preserves explicitly changed updated_at; otherwise, it updates the timestamp only when a non-excluded column differs, excluding updated_at, last_used_at, last_access_at, last_command_at, and next_auto_event_at.
Pre-merge checks: Passed 5

Check name Status Explanation
Title check Passed The title describes the main change and meets the required format.
Description Follows The Pr Template Passed The description contains all four required items: Summary, Validation, Severity and risk, and an AI assistance line.
Bug Fixes Name A Regression Test Passed The PR changes only database migrations. Its Validation section also names writing_the_idle_deadline_leaves_updated_at_alone and states that it fails without the migration and passes with it.
Weakened Safeguards Are Justified Passed The diff adds two database migration files. It does not weaken tests, assertions, lints, CI checks, or repository guards.
Public-Safe Content Passed The diff, title, and description contain no content prohibited by this check.
Recent review info

Automatic approval policy

Commit: 574d29d2cee7.

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

Commits

Reviewing files that changed from the base of the PR and between ff5a12a and 574d29d.

Files selected for processing: 2

  • db/postgres/20261106045040_sandboxes_updated_at_ignores_idle_deadline.down.sql
  • db/postgres/20261106045040_sandboxes_updated_at_ignores_idle_deadline.up.sql

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

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant