You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
template-mgr-store: add a store that owns template-manager's node_dirty_ref_revision - #1429
Adds pplx-space-template-mgr-store (crates/l30_storage/template/mgr/store), a storage-layer crate whose [package.metadata.space.state] declares sqlite.template-manager = ["node_dirty_ref_revision"]. archcheck requires every live table to have an owning store, and a new table can't be added to [layers.unowned]. So the migration that creates this counter (#1235) needs the declaration to merge first.
The crate owns only that table; template-manager's other tables stay baselined. It has no functions or SQL yet. Its first statements arrive with their first caller in #1235, which also adds template-manager's dependency and the [rules.layers.store_users] entry.
cargo fmt --check, and workspace cargo check and cargo clippy -D warnings with all targets and features: clean.
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --all-features for this crate, pplx-space-template-mgr and pplx-space-sqlite: 324/326 pass. The two failures are template-mgr's NoCoW tests, which need CI's btrfs fixture (ci/scripts/with-nocow-test-filesystem.sh); the validation host has no btrfs. They were not validated here.
Severity and risk
Severity: does not apply; no runtime change.
Risk: none at runtime, since nothing depends on the crate yet. Roll back by reverting, as long as #1235 has not merged on top.
No actionable comments were generated in the recent review.
Walkthrough
The workspace now includes the pplx-space-template-mgr-store crate and defines it as a workspace dependency. Its manifest declares SQLite template-manager state. Crate documentation describes node_dirty_ref_revision, its use by node_dirty_refs triggers and acknowledgement, and notes that the crate has no statements. The crate also forbids unsafe code.
Possibly related PRs
perplexityai/SPACE#1235: Adds the migration and revision-based outbox reads and acknowledgements to this store.
perplexityai/SPACE#1277: Adds rollback metadata and down SQL for the migration that creates the node_dirty_ref_revision table.
Unblocks: 1 PR
Merge Risk: Minimal · up to fa1dc
This registration-only change adds no runtime behavior or merge-blocking risk.
Security architecture risk: Minimal · up to fa1dc
The change registers a dependency-free storage crate without adding executable storage operations or runtime callers. The existing acknowledgement behavior is unchanged; no material security risk was found to be introduced or worsened.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The introduced scope is workspace registration and declared ownership of one node-local SQLite counter. The complete comparison shows no added service caller, external dependency, credential handling, or deployment configuration that would expand runtime exposure.
Security Findings and Attack Paths
inferred — The existing worker can delete a newer obligation if the same reference is re-enqueued after selection but before successful acknowledgement, because deletion matches only the reference key. This race predates the PR and is not worsened by registration. The scoped evidence does not establish an attacker-controlled exploit or privilege gain.
Resilience and Maintainability Implications
observed — The unchanged worker retains persisted obligations on send failure and retries with backoff. A successful send followed by failed deletion or interruption can result in a repeat send. These recovery paths remain outside the new store; remote endpoint idempotency was not established by this inspection.
Pre-merge checks: 5
Check name
Status
Explanation
Title check
The title does not follow the required format. It starts with "template-mgr-store:", which lowercase area prefix and colon match the rule. The summary after the colon starts with the lowercase word "a…
Description Follows The Pr Template
The PR description contains all required sections with real content: “## Summary” explains the change; “## Validation” lists commands and results; “## Severity and risk” states that severity does not …
Bug Fixes Name A Regression Test
The pull request is not an application bug fix. Its diff adds a workspace crate, Cargo metadata, lockfile registration, and crate documentation only; it adds no application behavior under crates/, `…
Weakened Safeguards Are Justified
The pull request adds a workspace member, a package manifest, metadata, and crate documentation. The diff does not remove, disable, narrow, or weaken tests, assertions, lints, CI checks, or repository…
Public-Safe Content
The reviewed diff adds only public package metadata, a crate manifest, and documentation. Added lines contain no credentials, private keys, internal URLs or hostnames, cloud identifiers, personal abso…
Full details: Title check
Explanation
The title does not follow the required format. It starts with "template-mgr-store:", which lowercase area prefix and colon match the rule. The summary after the colon starts with the lowercase word "add". It names the store and the owned counter, node_dirty_ref_revision, which the changeset defines. No trailing period, ticket ID, or link appears. The title meets the area-prefix requirement and describes the main change accurately.
Full details: Description Follows The Pr Template
Explanation
The PR description contains all required sections with real content: “## Summary” explains the change; “## Validation” lists commands and results; “## Severity and risk” states that severity does not apply and gives the runtime risk; and “AI assistance:” identifies Claude Code and the drafted content.
Full details: Bug Fixes Name A Regression Test
Explanation
The pull request is not an application bug fix. Its diff adds a workspace crate, Cargo metadata, lockfile registration, and crate documentation only; it adds no application behavior under crates/, shim/, or ui/. The regression-test requirement does not apply.
Full details: Weakened Safeguards Are Justified
Explanation
The pull request adds a workspace member, a package manifest, metadata, and crate documentation. The diff does not remove, disable, narrow, or weaken tests, assertions, lints, CI checks, or repository guards. The new [lints] workspace = true setting inherits existing workspace lints.
Full details: Public-Safe Content
Explanation
The reviewed diff adds only public package metadata, a crate manifest, and documentation. Added lines contain no credentials, private keys, internal URLs or hostnames, cloud identifiers, personal absolute paths, production data, incident details, or undisclosed vulnerability details. The title and description contain no prohibited sensitive content; issue references are ordinary public references.
Recent review info
Automatic approval policy
Commit: fa1dc7fe43f2.
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):
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.
Reviewing files that changed from the base of the PR and between a466def and fa1dc7f.
Files ignored due to path filters: 1
Cargo.lock is excluded by !**/*.lock, !**/Cargo.lock
Files selected for processing: 3
Cargo.toml
crates/l30_storage/template/mgr/store/Cargo.toml
crates/l30_storage/template/mgr/store/src/lib.rs
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 1 remain after this review.
…ty_ref_revision
template-manager's next migration creates node_dirty_ref_revision, the
node-wide counter its node_dirty_refs triggers stamp onto each enqueued row.
archcheck's table-ownership rule requires every live table to have an owning
store or a [layers.unowned] baseline entry, and a new table may not be
baselined, so that migration needs a store that declares the table first.
Add pplx-space-template-mgr-store under crates/l30_storage/template/mgr/store.
Its [package.metadata.space.state] declares
sqlite.template-manager = ["node_dirty_ref_revision"] and nothing else; the
other template-manager tables stay baselined. The crate has no functions or
SQL yet: its first statements arrive with their first caller, together with
the migration, so template-manager neither depends on it nor is listed in
[rules.layers.store_users] until then.
archcheck reports the declaration as the advisory layers.state.pending
finding until the migration lands, and no required findings.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Adds
pplx-space-template-mgr-store(crates/l30_storage/template/mgr/store), a storage-layer crate whose[package.metadata.space.state]declaressqlite.template-manager = ["node_dirty_ref_revision"]. archcheck requires every live table to have an owning store, and a new table can't be added to[layers.unowned]. So the migration that creates this counter (#1235) needs the declaration to merge first.The crate owns only that table; template-manager's other tables stay baselined. It has no functions or SQL yet. Its first statements arrive with their first caller in #1235, which also adds template-manager's dependency and the
[rules.layers.store_users]entry.Validation
At this head:
ci/scripts/run-archcheck.sh origin/main "": 0 required. 1 advisorylayers.state.pendingfor the declared table, expected until template-manager: ack node dirty refs by revision so a re-enqueue during a send survives #1235's migration lands.cargo fmt --check, and workspacecargo checkandcargo clippy -D warningswith all targets and features: clean.SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --all-featuresfor this crate, pplx-space-template-mgr and pplx-space-sqlite: 324/326 pass. The two failures are template-mgr's NoCoW tests, which need CI's btrfs fixture (ci/scripts/with-nocow-test-filesystem.sh); the validation host has no btrfs. They were not validated here.Severity and risk
Severity: does not apply; no runtime change.
Risk: none at runtime, since nothing depends on the crate yet. Roll back by reverting, as long as #1235 has not merged on top.
Stack
Merge in that order.
AI assistance: Claude Code drafted the crate, the commit message and this description.