[ac-pplx]

Summary

A single sandboxes row whose stored lifecycle is not canonical text no longer fails a node's whole PublishNodeState. The two steps that failed on such a row, pause-failed recovery and activation rollback, now skip it and leave its lifecycle as stored. The rest of the publication commits, including that row's observed state and activity clocks.

Example: a pause-failed quarantined row stored as ' Quarantined'. Pause-failed recovery selects candidates with LOWER(TRIM(s.lifecycle)) = 'quarantined', but the recovery write in pplx_space_sandbox_store::ingest matches lifecycle = 'quarantined' exactly. The row was selected, the write matched nothing, and the publication answered Aborted. That rolled back the node's whole publication, and every later one failed the same way until someone repaired the row. The activation-rollback candidate query had the same shape: it decoded LOWER(TRIM(s.lifecycle)) as a SandboxLifecycle, so a value no lifecycle names, such as bogus (the column has no CHECK), failed the publication with Internal.

Change (sandbox_quarantine_store.rs):

• Pause-failed recovery reads the stored lifecycle with each candidate and skips any row that does not parse as quarantined.
• Activation-rollback candidates read the prior lifecycle as text and omit the receipt of any row whose trimmed, lower-cased text names no lifecycle.
• Each skipped row logs a warning with the node and sandbox ids and counts in sandbox.state_publication.degraded with field:stored_lifecycle and disposition skip_pause_failed_recovery (recovery) or omit_transition_receipt (rollback receipt). stored_lifecycle and skip_pause_failed_recovery are new label values in pplx-space-metrics.
• Stored values are not rewritten and the write's predicate is unchanged. Any other miss of the pause-failed write still fails the publication, because that write may have applied in part.

Changed test expectations:

• lifecycle_writers::healthy_report_recovers_only_a_pause_failed_quarantine: stored " Quarantined " now publishes Ok, and the row and its quarantine are unchanged (was: publication Err, rolled back).
• lifecycle_writers::activation_rollback_receipt_converges_its_target_out_of_every_live_lifecycle: stored "bogus" now publishes Ok and the row is unchanged (was: publication Err, rolled back).
• New: published_state::node_publish_recovers_pause_failed_rows_past_a_non_canonical_one publishes a padded and a canonical pause-failed row in one PublishNodeState. The canonical row recovers. The padded row keeps its text and quarantine, and its observation records the published state.
• New: lifecycle_writers::activation_rollback_converges_past_a_receipt_skipped_for_its_stored_lifecycle publishes rollback receipts for a bogus and a starting row in one publication. The starting row converges; the bogus row keeps its lifecycle, placement, execution and port exposures.

Validation

At this head, rebased onto main after #1426 turned the two changed lifecycle_writers tests into rstest cases (this PR's edits are reapplied to those cases; the new rollback test is unchanged):

• cargo fmt --all -- --check, ci/scripts/run-archcheck.sh (0 findings) and cargo clippy --workspace --locked --all-targets --all-features -- -D warnings: pass.
• SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --all-features --no-fail-fast -p pplx-space-internal-api-server -p pplx-space-metrics, Postgres running: 479 run, 479 passed, 0 skipped. Each of this PR's tests takes under 0.4 s (ci/scripts/check-test-durations.py passes).
• With internal-api: parse the published sandbox status once at ingest (narrow behaviour change: non-ASCII spellings are unrecognized) #1411's code under them, all 7 cases of the changed and new tests fail for the expected reasons (Aborted: pause-failed recovery CAS failed, and Internal: ... unknown sandbox lifecycle 'bogus').

On an earlier revision of this change, before the crate layer move and #1426, with the same non-test code:

• With the rollback's continue replaced by return Ok(Vec::new()), only the new rollback test failed (converged row); restored, it passed.
• Fresh Kind cluster (SPACE_KIND_TELEMETRY_MODE=otel infra/space/kind/scripts/up.sh, every SPACE container at that commit), then scripts/tests/smoke-kind-typed.sh all with CI's admin-token file and cold-receipts directory: ECW 91/91 and native 42/42 passed, 0 warned or failed, 1h06m wall (ecw.cold_create_environment and local.telemetry_reaches_local_backends ran).

Not validated: an end-to-end Kind reproduction, which needs a hand-edited sandboxes row; the Postgres tests cover it.

Severity and risk

Severity: medium. One malformed row could stop a node's state publication indefinitely and leave its whole inventory stale.

Risk: low. Pause-failed recovery skips only selected rows that its exact-match write could not move. Activation rollback skips only rows whose trimmed, lower-cased lifecycle names no lifecycle. Its writes would match those rows, but it needs the prior lifecycle, so the receipt is not applied: the row keeps its lifecycle, its active execution and its port state. The node keeps publishing the receipt while its local row holds it, so a publication after the row is repaired can apply it while the receipt's execution and activation attempt are still active. Each skip logs a warning and counts in sandbox.state_publication.degraded instead of failing the publication. A dashboard or monitor filtering sandbox.state_publication.degraded by disposition sees the new values. Roll back by reverting. No schema or protocol change.

Rollout

Only internal-api changes. While old and new replicas run together, a node's publication over such a row fails on an old replica and commits on a new one, so it can alternate until the rollout completes. Nodes are unaffected.

Stack

Lands on top of #1411 (parse the published status once at ingest). Merge #1411 first; this branch holds only the one commit above it.

AI assistance: Claude Code drafted the code, tests and this description, and ran the validation.

[ac-pplx]

@coderabbitai full review --use-credits

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Activation-rollback selection now skips rows with invalid stored lifecycle values and records a publication metric instead of failing the selection. Pause-failed recovery checks for the exact Quarantined lifecycle and skips non-canonical values. Tests cover skipped rows alongside valid rows that still converge.

Possibly related PRs

• perplexityai/SPACE#1420: Implements stored-lifecycle skip behavior for activation rollback and pause-failed recovery.
• perplexityai/SPACE#1411: Defines published-status parsing and the canonical published_lifecycle contract used by node publication.

Merge Risk: Low · up to 6dfed

Publications can now continue past malformed lifecycle rows, which remain unchanged until repaired. Focused tests cover the behavior; merge risk is low.

Security architecture risk: Low · up to 6dfed

The change limits publication failures while preserving the inspected ownership and quarantine checks. No introduced security vulnerability was established. Successful publication can still leave malformed rows unrecovered, and eventual repair and cleanup are not fully demonstrated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The inspected change narrows malformed-row failure propagation from a node-wide publication failure to individual omitted transitions. Recovery mutations remain scoped to that node’s owned sandbox rows; no expanded cross-node authority was established.

Trust Boundaries and Controls

• observed — The unchanged endpoint resolves a registered origin and validates the connection’s remote IP against its registered address before publication. The new skip decision uses stored central lifecycle text, rather than granting authority based on a node-supplied lifecycle spelling.

Resilience and Maintainability Implications

• observed — Skipping occurs before the affected recovery writes. Eligible rollback candidates still couple fenced execution retirement, port-state invalidation and lifecycle convergence in the publication transaction. Other recovery write misses remain errors, preserving rollback of potentially partial writes rather than broadening the exception.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title uses the bracketed lowercase area prefix "[cr-test]" and a lowercase summary. It accurately describes skipping non-canonical stored lifecycles during internal API publication, with no traili…
Description Follows The Pr Template		The description contains all required non-empty items: a ## Summary section describing the merge behavior; a ## Validation section naming commands and passing results; a ## Severity and risk sec…
Bug Fixes Name A Regression Test		The pull request fixes application behavior in crates/. Its Validation section states that the changed and new tests pass with the fix and that all seven cases fail under the pre-fix #1411 code for …
Weakened Safeguards Are Justified		The diff changes four source/test files only. It adds no allow/skip/xfail, CI exception, lint relaxation, allowlist entry, or removed check. The lifecycle tests replace prior failure expectations with…
Public-Safe Content		No public-safety violation is present. The title, description, and diff contain no credentials, token values, private keys, internal URLs, cloud identifiers, personal absolute paths, production logs, …

Full details: Title check

Explanation

The title uses the bracketed lowercase area prefix "[cr-test]" and a lowercase summary. It accurately describes skipping non-canonical stored lifecycles during internal API publication, with no trailing period, ticket ID, or internal link.

Full details: Description Follows The Pr Template

Explanation

The description contains all required non-empty items: a ## Summary section describing the merge behavior; a ## Validation section naming commands and passing results; a ## Severity and risk section with severity and risk; and an AI assistance: line identifying Claude Code and the parts it drafted.

Full details: Bug Fixes Name A Regression Test

Explanation

The pull request fixes application behavior in crates/. Its Validation section states that the changed and new tests pass with the fix and that all seven cases fail under the pre-fix #1411 code for the expected publication errors. The diff includes regression coverage for padded Quarantined recovery, unknown lifecycle rollback, and mixed publications.

Full details: Weakened Safeguards Are Justified

Explanation

The diff changes four source/test files only. It adds no allow/skip/xfail, CI exception, lint relaxation, allowlist entry, or removed check. The lifecycle tests replace prior failure expectations with explicit success assertions and unchanged-row checks. The PR description gives a specific reason: malformed stored lifecycles must not abort the publication. The new mixed-row tests add coverage for the skipped and successfully processed rows.

Full details: Public-Safe Content

Explanation

No public-safety violation is present. The title, description, and diff contain no credentials, token values, private keys, internal URLs, cloud identifiers, personal absolute paths, production logs, or incident details. The operational references are generic test context, and values such as bogus and 127.0.0.1 are test data.

Recent review info

Automatic approval policy

Commit: 6dfed87a7458.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
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.

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: cr-test/1420-v5-head
• File: .coderabbit.yaml
• Commit: 6dfed87
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 24e3b911-2edc-49ac-999b-e3caf9089d7f

Commits

Reviewing files that changed from the base of the PR and between f572fb3 and 6dfed87.

Files selected for processing: 4

• crates/l40_api/internal/api/server/src/published_state_tests/lifecycle_writers.rs
• crates/l40_api/internal/api/server/src/sandbox_quarantine_store.rs
• crates/l40_api/internal/api/server/tests/published_state.rs
• crates/libs/metrics/src/sandbox/publication.rs

Limit details: You’ve used all 12 included reviews currently available.

[github-actions]

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

CONTRIBUTING.md explains how we review outside contributions.

[coderabbitai]

Action performed

Full review finished.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 7 minutes.

[ac-pplx]

Closing: CodeRabbit configuration test finished; the tested configuration is in #1496.