[ac-pplx]

Summary

A single sandboxes row whose stored lifecycle is not canonical text no longer fails a node's whole PublishNodeState. Ingest skips that row, leaves it as stored, and commits the rest of the publication.

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 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.

Validation

At this head:

• cargo fmt --all -- --check, scripts/check-rust-error-policy.sh: pass. cargo arch against the base's baseline and ARCHITECTURE.toml: 0 required (pattern_ratchet.sandbox_writes unchanged). cargo clippy --workspace --locked --all-targets --all-features -- -D warnings: pass.
• The three tests above fail on the base (internal-api: parse the published sandbox status once at ingest (narrow behaviour change: non-ASCII spellings are unrecognized) #1411's head) for the expected reasons (Aborted: pause-failed recovery CAS failed, and Internal: ... unknown sandbox lifecycle 'bogus') and pass here.
• SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --all-features over pplx-space-internal-api-server, pplx-space-sandbox-store, pplx-space-metrics and their 32 direct dependents, Postgres running: 9672 run, 9618 passed, 54 failed, 22 skipped. All 54 are in node/guest crates and need root or the CI btrfs NoCoW fixture. The same 54 fail on the base, and none is in a changed package.
• Fresh Kind cluster from this head (SPACE_KIND_TELEMETRY_MODE=otel infra/space/kind/scripts/up.sh, every SPACE container at this 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, 1h07m 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.

• After the crate layer move (crates: move every crate into its layer directory #1368): rebased onto internal-api: parse the published sandbox status once at ingest (narrow behaviour change: non-ASCII spellings are unrecognized) #1411's moved head c72e0f88 as 813ef1d4. The git range-diff differs only in file paths. cargo fmt, cargo check --workspace --all-targets --all-features, cargo arch and clippy pass, and SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --all-features -p pplx-space-db-test-fixtures -p pplx-space-sandbox-store -p pplx-space-metrics -p pplx-space-internal-api-server passed 507/507, including the three changed and new tests. The Kind run above was on the pre-move head, whose code is identical.

Severity and risk

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

Risk: low. Only rows the write could not move are skipped, and they stay as stored, now with a warning and a counter instead of a failed 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

State publication now skips activation rollback receipts with unparseable stored lifecycles and skips pause-failed recovery for non-canonical quarantined values. Skipped rows produce publication degradation metrics. Tests verify that publication commits, canonical quarantine recovery proceeds, and padded lifecycle text and its quarantine remain unchanged.

Possibly related issues

• Enforce retained-suspend erase safety across delete and eviction with operator release #876 — This change prevents some non-canonical lifecycle values from aborting state publication, but it does not implement the issue’s broader quarantine and deletion policy.

Possibly related PRs

• perplexityai/SPACE#1420 — The PR description reports the same lifecycle skip behavior and overlapping code and tests.
• perplexityai/SPACE#1411 — It changes the canonical lifecycle parsing and recovery selection that this PR extends.
• perplexityai/SPACE#1553 — It changes the stored-lifecycle enum and activation rollback candidate selection used by this PR.

Merge Risk: Moderate · up to fb075

Malformed lifecycle rows defer recovery while other sandbox updates commit. This reconciliation change carries moderate merge risk.

Security architecture risk: Low · up to fb075

Malformed stored lifecycle values no longer block a node's entire update. Ownership checks and quarantine protections remain in place, but skipped records remain unrepaired even when the update succeeds.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The changed recovery paths remain restricted to sandboxes assigned to the reporting node. Activation rollback additionally requires the active execution generation and activation-attempt identity; pause-failed recovery requires the matching quarantine source and reason. The PR does not broaden these selection predicates.

Trust Boundaries and Controls

• observed — The RPC still resolves the supplied node identifier through the existing registered-node, role, and remote-address checks, then passes the resolved origin identity into publication. The changed recovery functions remain internal helpers rather than newly exposed RPCs.

Resilience and Maintainability Implications

• inferred — Skipping a rollback candidate does not itself discard the node's receipt. Later publications can resend it while local state remains at the receipt target. Retention is not indefinite: an existing node-side write to a different lifecycle can clear it. That conditional retention also existed when malformed rows aborted publication, so it is not established as a new lost-transition path introduced by this PR.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title uses the lowercase bracketed area "[cr-test]" and a lowercase, accurate summary. It describes skipping non-canonical stored lifecycles instead of aborting node publication. It has no trailin…
Description Follows The Pr Template		The description contains all required sections and content: ## Summary explains the merge behavior; ## Validation lists commands and results; ## Severity and risk gives severity medium and ris…
Bug Fixes Name A Regression Test		The PR fixes application behavior under crates/. The description's Validation section states that the three regression tests fail on the base and pass on this branch. The named tests are `activation…
Weakened Safeguards Are Justified		No weakened safeguard matches the check. The diff modifies two existing expectations because the described behavior changes from publication failure to skipping malformed lifecycle rows. The replaceme…
Public-Safe Content		No public-safety violation found. The reviewed diff contains only lifecycle values, test data, and a loopback test address. The title and description contain no credentials, private keys, internal URL…

Full details: Title check

Explanation

The title uses the lowercase bracketed area "[cr-test]" and a lowercase, accurate summary. It describes skipping non-canonical stored lifecycles instead of aborting node publication. It has no trailing period, ticket ID, or internal link.

Full details: Description Follows The Pr Template

Explanation

The description contains all required sections and content: ## Summary explains the merge behavior; ## Validation lists commands and results; ## Severity and risk gives severity medium and risk low; and AI assistance: identifies Claude Code and states that it drafted the code, tests, description, and validation.

Full details: Bug Fixes Name A Regression Test

Explanation

The PR fixes application behavior under crates/. The description's Validation section states that the three regression tests fail on the base and pass on this branch. The named tests are activation_rollback_receipt_converges_its_target_out_of_every_live_lifecycle, healthy_report_recovers_only_a_pause_failed_quarantine, and node_publish_recovers_pause_failed_rows_past_a_non_canonical_one. The diff confirms these tests cover malformed stored lifecycle values and successful publication.

Full details: Weakened Safeguards Are Justified

Explanation

No weakened safeguard matches the check. The diff modifies two existing expectations because the described behavior changes from publication failure to skipping malformed lifecycle rows. The replacement checks still require successful publication and exact row/exposure state, and a new integration test adds coverage for mixed canonical and padded rows. The diff adds no skip/xfail, allowlist, lint, CI, nextest, or test-configuration exception.

Full details: Public-Safe Content

Explanation

No public-safety violation found. The reviewed diff contains only lifecycle values, test data, and a loopback test address. The title and description contain no credentials, private keys, internal URLs or hostnames, cloud identifiers, personal absolute paths, production logs, or undisclosed security-incident details.

Recent review info

Automatic approval policy

Commit: fb0759915d1e.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Moderate	Global configuration
Security risk	Not set	Global configuration

No overrides matched. The global limits apply.

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-r1-mod3-head
• File: .coderabbit.yaml
• Commit: fb07599
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: ed0c398c-c9ad-40c0-ba11-780e05c9e009

Commits

Reviewing files that changed from the base of the PR and between 06dc412 and fb07599.

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 review evaluation finished.