Skip to content

[V2:03] Add Repository Manifest Identity#17

Closed
ritorhymes wants to merge 1 commit into
eips-wg:masterfrom
ritovision:v2/03-repo-manifest-identity
Closed

[V2:03] Add Repository Manifest Identity#17
ritorhymes wants to merge 1 commit into
eips-wg:masterfrom
ritovision:v2/03-repo-manifest-identity

Conversation

@ritorhymes

@ritorhymes ritorhymes commented May 5, 2026

Copy link
Copy Markdown
Contributor

Part of V2: Multi-repo local build system.

Summary

This PR adds the manifest-backed repository identity layer that later workspace lifecycle and execution-policy PRs consume. It introduces the .build-eips.repo.toml schema with validation and an ActiveRepoIdentity selector that prefers a manifest when present and falls back to legacy EIPs/ERCs config; nothing in main.rs calls this yet, so review should focus on schema rules and fallback semantics rather than integration.

  • Add the .build-eips.repo.toml schema for active proposal repositories and declared sibling repositories.
  • Add manifest loading, validation, parse errors, and schema tests.
  • Add ActiveRepoIdentity for selecting either manifest-backed identity or the legacy EIPs/ERCs fallback.
  • Add the tempfile dev dependency used by manifest tests.

Rationale

The manifest is tracked proposal-repo identity and sibling topology, not general local fork configuration. The existing identifying_commit fallback fits the legacy EIPs/ERCs-only architecture, but the local build system needs repo-owned identity data once it operates inside a multi-repo workspace.

Keeping this in tracked repo metadata lets build, serve, init, doctor, and execution-policy resolution identify the active proposal repo and its sibling repos without hardcoding every repo relationship into the preprocessor binary. Theme remains shared workspace infrastructure at workspace/theme, and local execution preferences remain in .build-eips.toml.

Review Notes

This PR defines the manifest-backed identity model; it does not complete adoption by itself. The EIPs/ERCs rollout PRs (V2:20 and V2:21) add the tracked .build-eips.repo.toml files that make those repos use this path. Until a repo carries that manifest, the legacy EIPs/ERCs fallback remains available.

Later workspace lifecycle PRs (V2:06-V2:07) and execution policy V2:08 consume this layer to identify the active repo, discover sibling repos, and resolve repository metadata.

Review config.rs for the manifest schema and validation rules. Review identity.rs for the fallback behavior: repositories with a manifest use that metadata, while existing EIPs/ERCs-style repositories can still resolve through legacy config.

ActiveRepoIdentity::load is defined here but not yet called from main.rs; the workspace lifecycle and execution policy PRs wire it in. Manifest-backed accessors such as RepoManifest::sibling_repositories are also foundation in this slice and are consumed by those later PRs.

Security note: identity assertion shifts from “the binary recognizes you by commit content” to “the repo declares its identity in tracked metadata.” The security boundary is the same in both cases: you trust the checkout you ran the preprocessor on. The new model is explicit and reviewable rather than implicit, and reserved repo ids (theme, preprocessor, eipw) prevent shadowing workspace infrastructure.

Verification

  • Review src/config.rs for .build-eips.repo.toml parsing, validation, and tests.
  • Review src/identity.rs for manifest-backed identity selection and legacy fallback behavior.
  • Review the manifest validation tests for reserved repo ids, safe repo keys, self-sibling rejection, and duplicate sibling repository detection.
  • Review Cargo.toml and Cargo.lock for the tempfile test dependency.

@ritorhymes ritorhymes changed the title Add Repository Manifest Identity [V2:03] Add Repository Manifest Identity May 5, 2026
@ritorhymes ritorhymes force-pushed the v2/03-repo-manifest-identity branch 2 times, most recently from bb2c3ee to 786ba1e Compare May 9, 2026 20:10

@SamWilsn SamWilsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this makes the preprocessor very configurable. Do we need this level of complexity?

Is the point to allow someone to point the preprocessor at their own forks of our repositories? I'm just struggling a bit to see what the use case for this will be.

Comment thread src/config.rs Outdated
#[derive(Debug, Snafu)]
pub enum RepoManifestError {
#[snafu(display("i/o error while accessing `{}`", path.to_string_lossy()))]
RepoFs {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RepoFs {
Io {

Comment thread src/config.rs Outdated
"unable to parse repo manifest `{}`",
manifest_path.to_string_lossy()
))]
RepoParse {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
RepoParse {
Parse {

Comment thread src/config.rs Outdated
}
}

fn required_manifest_value<T>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a member of RepoManifest (unless it's used elsewhere and I missed it).

Comment thread src/config.rs
Comment thread src/config.rs Outdated
Comment on lines +155 to +161
pub fn active_endpoint(&self, staging: bool) -> RepositoryEndpoint {
if staging {
self.staging.clone()
} else {
self.production.clone()
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pub fn active_endpoint(&self, staging: bool) -> RepositoryEndpoint {
if staging {
self.staging.clone()
} else {
self.production.clone()
}
}
pub fn active_endpoint(&self, staging: bool) -> &RepositoryEndpoint {
if staging {
&self.staging
} else {
&self.production
}
}

Unless there's a reason not to?

Comment thread src/config.rs Outdated
})
}

fn validate_repo_key(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. The validate functions should be private members of the struct that needs them.

Add the .build-eips.repo.toml schema, loader, validation rules, and
manifest tests for active proposal repositories and declared sibling
repositories.

Introduce ActiveRepoIdentity so later workspace lifecycle and execution
layers can select manifest-backed repository metadata while the legacy
EIPs/ERCs fallback continues to operate.
@ritorhymes ritorhymes force-pushed the v2/03-repo-manifest-identity branch from 786ba1e to 9fec377 Compare May 26, 2026 01:02
@ritorhymes

ritorhymes commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

So this makes the preprocessor very configurable. Do we need this level of complexity?

Is the point to allow someone to point the preprocessor at their own forks of our repositories? I'm just struggling a bit to see what the use case for this will be.

This is not meant to make arbitrary forks first-class through local config. It is meant to move proposal-repo identity and sibling topology out of hardcoded preprocessor config and into tracked repo metadata, because the local build system needs that information to operate inside a multi-repo workspace. Theme is intentionally kept out of this and remains shared workspace infrastructure at workspace/theme; local execution preferences stay in .build-eips.toml.

The current identifying_commit system fits the old architecture because the preprocessor had a small, fixed universe: EIPs and ERCs. The binary could centrally know both repos, their URLs, their base URLs, and a commit unique enough to identify each one without requiring repo-owned metadata.

The limitation shows up once the local build system lands, because the tool stops being just “render one of two known upstream repos” and becomes “operate inside a multi-repo workspace.” At that point it needs to know the active repo, sibling proposal repos, and staging/production endpoints so build, serve, init, doctor, and execution-policy resolution can derive the right workspace layout and repository sources. Hardcoded binary config becomes friction there because every new proposal repo or split requires a code change and release before the local tooling can identify and wire it correctly.

The current identifying_commit lookup also becomes brittle for history-preserving splits. If, for example, Core splits out of EIPs, the new repo could inherit the existing EIPs identifying commit; making the legacy path work would require a coordinated preprocessor update with fresh post-split identifying commits for the affected repos. A tracked manifest avoids that by letting the repo declare its identity directly, and it also lets the split be exercised locally before a preprocessor release knows about it.

So this is not general fork configurability; it is the narrow tracked-data alternative to hardcoding each proposal repo and sibling relationship into the binary. The trade-off is that, in a multi-repo workspace, either the preprocessor remains the central authority for every repo relationship, or each proposal repo carries the tracked metadata needed to declare its own identity and reconnect to the larger build system. This PR chooses the repo-owned metadata path so new or split proposal repos do not require a preprocessor release before they can be identified locally.

@ritorhymes

Copy link
Copy Markdown
Contributor Author

Pushed updates for the inline review items:

  • renamed the repo manifest error variants
  • moved manifest required-field and validation helpers onto RepoManifest
  • kept the manual required-field handling instead of adding optionable
  • changed active_endpoint to return a borrowed endpoint
  • rebased and pushed the dependent stack on top of this branch

I also updated the PR description with the rationale for the manifest layer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants