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

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Activation-rollback publication skips rows with unrecognized stored lifecycle values and continues processing eligible receipts. Pause-failed recovery skips rows whose stored lifecycle is not exactly Quarantined. Publication metrics record skipped rows. Tests verify that publication commits, eligible rows recover or converge, and skipped rows retain their stored state.

Possibly related PRs

• perplexityai/SPACE#1411: Introduces canonical published_lifecycle values used by this PR’s lifecycle parsing and recovery checks.

Merge Risk: Low · up to bf5f2

Malformed lifecycle rows no longer block publication, but remain unchanged until repaired. The change is mergeable with that operational awareness.

Security architecture risk: Low · up to bf5f2

The change isolates an invalid sandbox row without weakening the inspected ownership, receipt, or quarantine checks. No introduced security issue was established. Recovery after repairing an invalid row remains conditional on later observations and valid transition metadata and was not exercised end to end.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The changed transition paths remain limited to central sandbox rows assigned to the publishing node. Failure containment improves from blocking that node's publication to omitting the affected transition; the inspected change does not create cross-node mutation authority.

Trust Boundaries and Controls

• observed — Publication resolves the claimed node through registered node data and checks the remote address. Node-controlled sandbox identifiers, observations, and receipts then encounter central ownership and transition checks. Rollback requires matching execution identity; pause recovery remains restricted to the expected quarantine source and reason, with exact lifecycle and quarantine identity rechecked by storage.

Resilience and Maintainability Implications

• inferred — The change removes malformed stored lifecycle text as a publication-wide failure trigger without treating it as permission to bypass a transition guard. It preserves degraded rows for repair and makes the omission observable. Successful publication therefore does not mean every lifecycle transition converged.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title starts with the lowercase area "internal-api:" and uses a lowercase summary. It accurately describes the main publication behavior change and has no trailing period, ticket ID, or internal l…
Description Follows The Pr Template		The description contains all required sections and content: ## Summary explains the merge behavior; ## Validation names commands and successful results; ## Severity and risk states medium severi…
Bug Fixes Name A Regression Test		This is an application bug fix under crates/. The PR adds regression tests for both failure paths: activation_rollback_converges_past_a_receipt_skipped_for_its_stored_lifecycle and `node_publish_rec…
Weakened Safeguards Are Justified		The diff does not remove or weaken a repository guard, lint, CI check, allowlist, or test without justification. It changes two existing expectations from publication failure to successful skip behavi…
Public-Safe Content		No prohibited public-safety content is present. The diff contains code, synthetic lifecycle values, test IDs, localhost test addressing, and metric names only. The title and description contain no cre…

Full details: Title check

Explanation

The title starts with the lowercase area "internal-api:" and uses a lowercase summary. It accurately describes the main publication behavior change and 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 names commands and successful results; ## Severity and risk states medium severity and low risk; and the AI assistance: line identifies Claude Code and the drafted code, tests, description, and validation.

Full details: Bug Fixes Name A Regression Test

Explanation

This is an application bug fix under crates/. The PR adds regression tests for both failure paths: activation_rollback_converges_past_a_receipt_skipped_for_its_stored_lifecycle and node_publish_recovers_pause_failed_rows_past_a_non_canonical_one. The Validation section reports that all changed and new test cases fail with the pre-fix code and that the full nextest run passes with the fix.

Full details: Weakened Safeguards Are Justified

Explanation

The diff does not remove or weaken a repository guard, lint, CI check, allowlist, or test without justification. It changes two existing expectations from publication failure to successful skip behavior, with the specific reason stated in the PR description and adjacent test comments. The same diff adds replacement coverage: mixed activation-rollback receipts verify the malformed row stays unchanged while another row converges, and mixed pause-failed recovery verifies the canonical row recovers while the padded row retains its lifecycle and quarantine. No configuration or CI files changed.

Full details: Public-Safe Content

Explanation

No prohibited public-safety content is present. The diff contains code, synthetic lifecycle values, test IDs, localhost test addressing, and metric names only. The title and description contain no credentials, token values, private keys, internal URLs, tracker links, cloud IDs, real resource ARNs, personal absolute paths, incident details, or raw production data.

Recent review info

Automatic approval policy

Commit: bf5f2dae1df0.

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: ac/lifecycle-pause-failed-skip
• File: .coderabbit.yaml
• Commit: bf5f2da
• Layers: Repository YAML; Enforced overrides: Organization
• Review profile: CHILL
• Plan: Enterprise
• Run ID: b2beff96-589f-443d-9fc9-25d1b37ad3d3

Commits

Reviewing files that changed from the base of the PR and between 864947b and bf5f2da.

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

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 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]

@coderabbitai full review

[coderabbitai]

Action performed

Full review finished.

[ac-pplx]

@coderabbitai full review

[coderabbitai]

Action performed

Full review finished.