feat(compile): add compact repos: front-matter syntax with codemod auto-rewrite#478
Conversation
Replace the verbose `repositories:` + `checkout:` pair with a single
`repos:` block. Each entry can be a string shorthand ("org/repo" or
"alias=org/repo") or an object with name/alias/type/ref/checkout fields.
Checkout defaults to true. Legacy fields remain supported with a
deprecation warning; mixing the two is rejected at compile time.
Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c9ae0fbb-1f1c-4874-b990-16fd3b4bc3e3
Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
- Handle trailing slash in derive_alias (trim before splitting) - Add tests for reserved alias via explicit alias (shorthand & object) - Add test for empty name after '=' in shorthand - Add test for trailing slash alias derivation Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c9ae0fbb-1f1c-4874-b990-16fd3b4bc3e3 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
|
@copilot rebase on feat/frontmatter-migrations and implement a front matter migration for this breaking change. |
Introduces a Django/Rails-style schema-version field on FrontMatter and a registered migration framework so breaking grammar changes auto-migrate existing user sources during compile rather than hard-failing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Address code review finding: the runner did `1 + registry.len() as u32` and `m.to_version == current + 1` without overflow checks. With realistic registries this is unreachable, but rust panics on overflow in debug mode and wraps in release. Switch to checked_add so we surface a clear error either way; preserves existing behavior on all realistic inputs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Four findings from the Rust PR Reviewer bot: 1. check_pipeline's bail message had a redundant "error:" prefix that produced "Error: error: ..." once anyhow's top-level handler added its own "Error:" wrapper. Drop the prefix and reformat the hint onto an indented continuation line. 2. compile_pipeline_inner did a redundant serde_yaml::from_value round-trip just to satisfy ParsedSource.front_matter, even though perform_source_rewrite_if_needed never reads that field. Refactor the helper to take the four primitive fields it actually uses (mapping, body_raw, source_sha256, migrations) instead of the full struct. reconstruct_source likewise takes individual fields now. 3. Misleading comment in tests/migration_tests.rs claimed a "thread-local counter" was used; only a nanosecond timestamp was. Replaced rand_suffix() with timestamp + AtomicU64 monotonic seq so parallel tests scheduled in the same nanosecond always get distinct dirs. 4. Leading whitespace before the opening `---` was silently stripped on migration rewrite. parse_markdown_detailed already tolerates it on read; capture it as a new ParsedSource.leading_whitespace field and emit it from reconstruct_source so byte-faithful preservation extends to whitespace prefixes (BOM-stripped editor blank lines, etc.). All 1369 tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
…gration framework Rebased on feat/frontmatter-migrations and added 0001_repos_unified migration so existing sources are auto-rewritten to the new compact `repos:` shape on compile. With the migration in place, the legacy fields are removed from FrontMatter; the typed deserializer now only accepts `repos:`. The migration runner ensures any source that still declares `repositories:`/`checkout:` is converted (and stamped to schema-version 2) before typed deserialization runs. Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c4473df6-4e6d-4a84-9fa7-74f55749dc78 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c4473df6-4e6d-4a84-9fa7-74f55749dc78 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Done in 9c0c7ba. Rebased onto Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
repos: front-matter syntaxrepos: front-matter syntax with auto-migration
|
@copilot Undo that last change, then rebase on feat/frontmatter-migrations and implement a front matter codemod for this breaking change. |
|
@copilot Undo that last change, then rebase on feat/frontmatter-migrations and implement a front matter codemod for this breaking change. |
|
@copilot Undo that last change, then rebase on main and implement a front matter codemod for this breaking change. |
Replace the verbose `repositories:` + `checkout:` pair with a single
`repos:` block. Each entry can be a string shorthand ("org/repo" or
"alias=org/repo") or an object with name/alias/type/ref/checkout fields.
Checkout defaults to true. Legacy fields remain supported with a
deprecation warning; mixing the two is rejected at compile time.
Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c9ae0fbb-1f1c-4874-b990-16fd3b4bc3e3
Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
- Handle trailing slash in derive_alias (trim before splitting) - Add tests for reserved alias via explicit alias (shorthand & object) - Add test for empty name after '=' in shorthand - Add test for trailing slash alias derivation Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/c9ae0fbb-1f1c-4874-b990-16fd3b4bc3e3 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
…heckout: rewrite Replaces the previous front-matter migration framework approach with the codemod framework merged in #476. The new 0001_repos_unified codemod detects the legacy repositories: + checkout: shape in raw YAML and rewrites it in place to the unified repos: shape before typed deserialization. Idempotent on already-current sources. Drops the typed repositories: / checkout: fields from FrontMatter (now #[serde(skip)] Rust-only fields populated by lower_repos); the codemod runs first so user YAML never reaches typed deserialization with legacy keys. resolve_repos is simplified accordingly — the codemod owns the mixing/legacy concerns. Updates docs/front-matter.md to point at docs/codemods.md. Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Done in 6058baa. Reset onto Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…migrations) Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/32568271-c9c7-4a9c-a0f8-37d10b112446 Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
repos: front-matter syntax with auto-migrationrepos: front-matter syntax with codemod auto-rewrite
|
/rust-review |
|
✅ Rust PR Reviewer completed successfully! |
🔍 Rust PR ReviewSummary: Looks good overall — solid design and strong test coverage. One missing sanitization call and a minor codemod edge case worth addressing. Findings🐛 Bugs / Logic Issues
✅ What Looks Good
|
|
@copilot implement bug fixes |
…ing, no-op on empty repositories:) Agent-Logs-Url: https://github.com/githubnext/ado-aw/sessions/512194bf-d86b-457b-b919-752efb1eff3d Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Addressed rust-review bugs in da9d474:
Full suite (1419 tests) passes. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Summary
Declaring N additional repositories currently requires writing each one twice — once under
repositories:and again undercheckout:. This gets verbose and error-prone at scale. Borrowing from gh-aw's "one block per repo" principle, this adds a singlerepos:field that unifies both with sensible defaults (checkout: true,type: git,ref: refs/heads/main).Before (10 lines for 3 repos):
After (4 lines):
Changes
src/compile/types.rs—RepoEntrystruct,ReposItemenum (string shorthand or object) with customDeserialize, newreposfield onFrontMatter. The legacyrepositories:andcheckout:fields are removed from the typed deserializer (now#[serde(skip)]Rust-only fields populated bylower_repos).src/compile/common.rs—lower_repos(),parse_shorthand(),derive_alias()for loweringrepos:into the existing(Vec<Repository>, Vec<String>)internal representation.resolve_reposis a thin lowering call now that the codemod owns mixing/legacy concerns.src/compile/codemods/0001_repos_unified.rs— new front-matter codemod (registered in theCODEMODSregistry introduced in feat(compile): autorewrite front matter via detection-based codemods #476) that auto-rewrites old sources fromrepositories:+checkout:into the newrepos:shape in raw YAML before typed deserialization. Mapsrepository:→alias:, preservesname/type/ref, emits explicitcheckout: falseonly for entries that weren't in the originalcheckout:list, and carries unknown repository keys forward for forward-compat. Idempotent on already-current sources, rejects mixingrepos:with the legacy fields, and rejects danglingcheckout:aliases. Returns a true no-op (Ok(false)) when bothrepositories:andcheckout:are trivially empty, so the caller does not surface a "deprecated shapes" warning or rewrite the file just to drop empty stubs.src/compile/codemods/mod.rs/helpers.rs— wire the new codemod into theCODEMODSregistry; remove theTODO(codemods)placeholder annotations now that a real codemod has shipped; update helper docs to refer to "codemods".src/compile/mod.rs/src/main.rs— wirelower_repos(&fm.repos)into both compile paths and the execute command (which now usesparse_markdown_detailedso Stage 3 benefits from in-memory codemod fixes when a source has deprecated shapes).run_executenow callsfront_matter.sanitize_config_fields()beforeresolve_repos(), mirroringcompile_pipeline_innerandcheck_pipeline, so unsanitizedrepo.namevalues cannot flow intoctx.allowed_repositories.docs/front-matter.md— document the new syntax, object fields, examples, and a back-compat note pointing atdocs/codemods.md.test_fixture_complete_agentnow assertsrepos:; thecomplete-agent.mdfixture is normalized to the unifiedrepos:shape (the codemod rewrites it on first compile).Supported forms
Back-compat
Old sources are auto-rewritten by the front-matter codemod framework on the next
ado-aw compile— users see a warning, the file is updated in place, and the build moves on. The typed deserializer no longer accepts the legacy fields directly (the codemod runs first against the raw YAML mapping).Test plan
repos:lowering covering shorthand, object form,checkout: false,alias=syntax, duplicate alias rejection, reserved name rejection, trailing slash handling, and empty-name/empty-alias errors.0001_repos_unifiedcodemod plus an explicit idempotency test, covering full conversion, resource-only entries (repositories:withoutcheckout:), preservedref, mixing rejection, danglingcheckout:aliases, no-op on already-current shapes, trivially-emptyrepositories:returning a no-op, non-mapping rejection, and forward-compat carry-over of unknown keys.cargo test, 1419 tests).