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
types: move the template manifest and policy authority into Domain crates - #1702
Moves two more leaf modules out of pplx-space-common-core::types into Domain crates.
types::template::{manifest,metadata} → pplx-space-template-manifest
(crates/l20_domain/template/manifest): the typed manifest.json and metadata.json of a template build. Their serde-only cfg gates become
plain attributes, since the crate always uses serde.
types::data (PolicyAuthority) → pplx-space-policy-authority
(crates/l20_domain/policy/authority), unchanged, serde behind its own feature.
Every import is rewritten and common-core keeps no re-export. Tests move with
the code. No string literal, serde attribute, metric, span, log field, wire or
SQL text changes. kek, subject, sandbox_artifact and object_store
stay: they depend on SandboxId or TemplateName, which have not moved.
run-rust-checks.sh clippy, isolated-crates, tests-unprivileged (only the
known guest-process/node-mgr permission failures)
feature clippy for touched crates (api/controller/internal-api/node-ctl with
kubernetes; common-core all features; policy-authority with and without serde)
run-archcheck.sh against the base: no required findings, no baseline changes
Severity and risk
Severity: does not apply (refactor).
Risk: low. Only crate paths change. Code that imports these names through pplx_space_common_core::types must
import them from pplx_space_template_manifest or pplx_space_policy_authority. No serialized shape changes. Roll
back by reverting.
AI assistance: Claude Code drafted the code changes and this description.
No actionable comments were generated in the recent review.
Walkthrough
The workspace adds dedicated template-manifest and policy-authority crates. Template manifest and metadata definitions move out of common core, and affected build, sandbox, template-management, publishing, storage, API, and application code now uses the new crate paths. Manifest Serde derives and attributes are unconditional in the new crate. The supplied summaries report no changes to existing manifest validation or consumer control flow.
Possibly related PRs
perplexityai/SPACE#1696: This PR is stacked on #1696 and updates the same common-core type-module surface affected by its module removals.
Merge Risk: Minimal · up to c20a8
This is a mechanical crate extraction with imports rewritten and tests moved alongside the code. No concrete defects were identified, so merge risk is minimal.
Security architecture risk: Low · up to c20a8
This is a low-risk shared-contract relocation. The checked changes preserve document serialization, schema checks, and policy-authority semantics. Consumers must adopt the new crate paths. No new security weakness was identified in the inspected scope, but downstream coverage is incomplete.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The principal demonstrated propagation is through shared compile-time contracts used by template, build, and sandbox consumers. The reported dependent counts establish broad integration scope, not additional attacker-accessible endpoints or greater tenant, host, or datastore authority.
Trust Boundaries and Controls
observed — PolicyAuthority retains Cluster and Local semantics and the existing explicit protobuf conversion boundary. Unknown integers and required Unspecified values remain errors; the optional conversion still maps Unspecified to absence. The domain/protobuf enum distinction predates this PR rather than introducing a new authority interpretation.
Pre-merge checks: 6
Check name
Status
Explanation
Title check
The title uses the lowercase area prefix "types:" followed by a colon and a summary starting with a lowercase word, and it has no trailing period or ticket ID. It describes the main change: the templa…
Description Follows The Pr Template
The PR description contains all four required items, each with real content: 1. "## Summary" is present. It states what merging does: it moves types::template::{manifest,metadata} to `pplx-space…
Bug Fixes Name A Regression Test
The check applies only when the pull request fixes a defect in application behavior under crates/, shim/, or ui/. This pull request is not a bug fix. Evidence: - The title reads "types: move the templ…
Weakened Safeguards Are Justified
PR #1702 (github.com/perplexityai/SPACE, author ac-pplx) moves types::template::{manifest,metadata} and types::data (PolicyAuthority) out of pplx-space-common-core into two new Domain crates. I …
Agent Tooling Runs No New Commands
The check applies to agent plugin manifests, SKILL.md files, command files, and guideline files. This PR is a Rust crate refactor, and its diff contains no plugin manifests (plugin.json/plugin.yaml), …
Public-Safe Content
The Public-safe content check passes. I scanned all added lines in the PR range (74 changed files, about 440 added lines) for the failure categories in the check. - Credentials, tokens, and private ke…
Full details: Title check
Explanation
The title uses the lowercase area prefix "types:" followed by a colon and a summary starting with a lowercase word, and it has no trailing period or ticket ID. It describes the main change: the template manifest and policy authority types move out of common-core into the Domain crates (pplx-space-template-manifest and pplx-space-policy-authority), which matches the raw summary and the PR objectives.
Full details: Description Follows The Pr Template
Explanation
The PR description contains all four required items, each with real content: 1. "## Summary" is present. It states what merging does: it moves types::template::{manifest,metadata} to pplx-space-template-manifest and types::data (PolicyAuthority) to pplx-space-policy-authority. 2. "## Validation" is present. It names the commands run (cargo fmt --check, cargo check --workspace --all-targets, run-rust-checks.sh clippy, isolated-crates and tests-unprivileged, feature clippy, run-archcheck.sh). It also reports results, including the known guest-process/node-mgr permission failures. 3. "## Severity and risk" is present. It gives "Severity: does not apply (refactor)" and "Risk: low", with a rationale. 4. "AI assistance:" line is present: "AI assistance: Claude Code drafted the code changes and this description." The check tests presence only, so no code evidence was needed. The description text was supplied in full. The PR is #1702 by ac-pplx. The check does not depend on author identity.
Full details: Bug Fixes Name A Regression Test
Explanation
The check applies only when the pull request fixes a defect in application behavior under crates/, shim/, or ui/. This pull request is not a bug fix. Evidence: - The title reads "types: move the template manifest and policy authority into Domain crates". The two commit subjects are both moves: "move template::manifest and template::metadata out of common-core" and "move PolicyAuthority out of common-core". - The PR description calls the change a refactor ("Severity: does not apply (refactor)"). It states that only crate paths change, that no serialized shape changes, and that no string literal, serde attribute, metric, span, log field, wire format, or SQL text changes. - The raw summary lists no behavior change. Each file entry shows import-path rewrites and feature-gate changes. The summary says the public declarations changed only by type path. It describes no defect correction. The pull request does not fix an application defect, so the regression-test requirement does not apply. The check passes. I did not inspect the patch line by line. The verdict rests on the title, commit subjects, PR description, and summary, which all agree that this is a move-only refactor.
Full details: Weakened Safeguards Are Justified
Explanation
PR #1702 (github.com/perplexityai/SPACE, author ac-pplx) moves types::template::{manifest,metadata} and types::data (PolicyAuthority) out of pplx-space-common-core into two new Domain crates. I checked the full committed diff (base refs/coderabbit/pre-merge-check/base to head) against each item in the check's failure list. No item matches. - Lint and allow attributes: No #[allow(...)] or #[expect(...)] lines are added. No lint level is lowered in any Cargo.toml. The two new crates add [lints] workspace = true, which inherits lints rather than removing them. - Tests and assertions: Zero assert lines are removed and zero are added. Test bodies in the moved files change only in import paths (for example crate::types::RuntimeKind becomes pplx_space_execution_spec::RuntimeKind). No #[test] is removed, and none is marked #[ignore], skip or xfail. - Test gates: Two inline test modules change from #[cfg(all(test, feature = "serde"))] to #[cfg(test)] (manifest.rs and metadata.rs). The integration test template_manifest_api.rs drops #![cfg(feature = "serde")]. These gates broaden test compilation. They do not disable any test. The crate has a non-optional serde dependency, so the feature condition is always true there. - CI, allowlists, and config: No changes to nextest config, CI workflows, deny.toml, clippy.toml, .gitleaks.toml, ARCHITECTURE.toml, baselines, known-failures files, or kind-pull-paths.txt. No SPACE_TEST_DEPS_OPTIONAL change. - Rule and guidance files: The only change to docs/rust/space-conventions.md updates two example paths. The balloon.rs example it now points to does contain #[cfg(all(test, feature = "serde"))] at line 277. No AGENTS.md, CLAUDE.md, CONTRIBUTING.md or .agents/ file changes. No review or merge-risk guidance text is added. - Other cfg changes: Removed #[cfg(feature = "serde")] gates on moved serde derives and re-exports. These are part of the move. The serde-gated re-exports in types/mod.rs and types/template/mod.rs are removed, and no test is affected. Note, outside this check's failure criteria: the new #[cfg(test)] gates on the moved test modules do not follow the wording of the docs/rust/space-conventions.md "cfg attributes move verbatim" section. That section discourages the simplification. Because the check fails only on removed or loosened rules and does not edit that text, this does not change the verdict.
Full details: Agent Tooling Runs No New Commands
Explanation
The check applies to agent plugin manifests, SKILL.md files, command files, and guideline files. This PR is a Rust crate refactor, and its diff contains no plugin manifests (plugin.json/plugin.yaml), no SKILL.md or command files, and no agent configuration. The only guideline-type file changed is docs/rust/space-conventions.md. Its two hunks (around lines 324 and 362) replace file paths inside prose and inline code spans with new crate paths. Neither hunk adds a shell substitution, hook, MCP server, LSP server, or frontmatter key. A scan of all added lines for ! followed by a backtick, hooks, mcpServers, mcp_servers, lspServers, allowed-tools, and command: returned no matches. The only other non-Rust change is a docstring path update in scripts/lib/manifest_execution_to_proto.py, which is not an agent file. No agent tooling entry is added or changed, so the check passes.
Full details: Public-Safe Content
Explanation
The Public-safe content check passes. I scanned all added lines in the PR range (74 changed files, about 440 added lines) for the failure categories in the check. - Credentials, tokens, and private keys: no matches for token, secret, password, credential, API-key, AWS key, or private-key patterns in added lines. - Absolute workstation paths (/Users/, /home/): none. - Internal hostnames, internal URLs, and private tracker, chat, or document links: no http(s) URLs were added. The only "internal" and "cluster" hits are crate directory names in diff headers (for example l40_api/internal/api/server, l40_api/cluster/controller/server), which are repository paths, not hosts or environments. One doc-comment hit mentions "private helpers" in generic prose. - Cloud account IDs or ARNs: no 12-digit numbers or arn: strings in added lines. - Real cluster or environment names, production logs, or incident details: none found. The "prod" search had no added-line hits. - Security vulnerability descriptions: none. The PR is a refactor that moves modules between crates. The PR title and description contain no URLs, hostnames, paths, or sensitive values. The check fails only on sensitive content, and none was found.
Recent review info
Automatic approval policy
Commit: c20a8f361373.
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.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 6 remain after this review.
`types::template::manifest` and `types::template::metadata`, the typed
`manifest.json` and `metadata.json` documents of a template build, become
`pplx-space-template-manifest` (crates/l20_domain/template/manifest). They
belong together: each is the other's companion document, and the metadata
tests build a manifest. No other `types` module and nothing else in
common-core uses them, and they have no sqlx or rusqlite codec. Their users
are build-mgr, sbx-mgr, template-mgr and template-publisher.
Both modules were compiled only with common-core's `serde` feature, so the
new crate depends on serde unconditionally: each
`#[cfg_attr(feature = "serde", X)]` becomes `#[X]` with X unchanged, and the
test modules' `#[cfg(all(test, feature = "serde"))]` becomes `#[cfg(test)]`.
Paths into `types` are rewritten to `pplx-space-execution-spec`, which owns
those types, and the tests' `Arch` and `Digest` come from cpu-arch and
content-digest. The unit tests and the `template_manifest_api` integration
test move with the code. Every import is rewritten to the new crate, and
common-core keeps no re-export. No string literal, serde attribute, metric,
span or wire text changes.
`types::data`, the cluster-or-local authority marker for policy data, becomes
`pplx-space-policy-authority` (crates/l20_domain/policy/authority):
PolicyAuthority and PolicyAuthorityParseError, with serde behind the crate's
own `serde` feature as before. No other `types` module uses it, and it has
no sqlx or rusqlite codec: sbx-mgr-store reads and writes it as text through
`as_str` and `FromStr`. The file moves unchanged.
Its users are api, build-mgr, internal-api-server, node-ctl-server,
sbx-mgr-store, template-mgr, sbx-mgr-cli, template-mgr-cli, node-api-sim,
common-core's `k8s` module, and the tests of cluster-controller-server and
sbx-mgr, which take it as a dev-dependency. Only node-api-sim serializes it,
so only node-api-sim enables `serde`; each user was checked alone.
common-core's `k8s` module compiles only with its `kubernetes` feature, so
common-core's dependency is optional and that feature turns it on, as it
does base64, k8s-openapi and sha2. Every import is rewritten to the new
crate, and common-core keeps no re-export. No string literal, serde
attribute, metric, span or wire text changes.
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}
shehan-pplx
left a comment
Summary
Moves two more leaf modules out of
pplx-space-common-core::typesinto Domain crates.types::template::{manifest,metadata}→pplx-space-template-manifest(crates/l20_domain/template/manifest): the typed
manifest.jsonandmetadata.jsonof a template build. Their serde-only cfg gates becomeplain attributes, since the crate always uses serde.
types::data(PolicyAuthority) →pplx-space-policy-authority(crates/l20_domain/policy/authority), unchanged, serde behind its own feature.
Every import is rewritten and common-core keeps no re-export. Tests move with
the code. No string literal, serde attribute, metric, span, log field, wire or
SQL text changes.
kek,subject,sandbox_artifactandobject_storestay: they depend on
SandboxIdorTemplateName, which have not moved.Stacked on #1696.
Validation
known guest-process/node-mgr permission failures)
kubernetes; common-core all features; policy-authority with and without serde)
Severity and risk
Severity: does not apply (refactor).
Risk: low. Only crate paths change. Code that imports these names through
pplx_space_common_core::typesmustimport them from
pplx_space_template_manifestorpplx_space_policy_authority. No serialized shape changes. Rollback by reverting.
AI assistance: Claude Code drafted the code changes and this description.