Repository navigation
feat(scripts): detect modified-but-unreleased dependencies during release - #436
Merged
Sander Saares (sandersaares) merged 119 commits intoJun 10, 2026
Merged
Conversation
…lease When releasing a crate, an author may have also modified one of its upstream workspace dependencies but forgotten to release the dependency too. Locally everything builds via path-references, but once published the released crate resolves to the last released version on crates.io, missing the new changes. This adds two layers of automation: 1. Interactive layer (release-crate.ps1): After the existing downstream cascade finishes, scan the release set for transitive workspace dependencies that have file changes vs the PR base ref but are not themselves being released. Prompt the author (y/N + bump kind) for each finding so material changes get an extra release queued, while immaterial changes (formatting, doc tweaks) can be declined. 2. CI layer (scripts/check-unreleased-dependencies.ps1 + .github/workflows/main.yml release-deps job): Runs the same analysis non-interactively and posts a sticky PR comment listing any findings so reviewers can sanity-check materiality decisions. Shared logic lives in the new scripts/lib/releasing.ps1 library, dot-sourced by both entry-point scripts. Workspace dependency types kind=normal and kind=build are tracked; kind=dev is excluded (cannot affect downstream consumers via crates.io). Cascade re-bump is idempotent: re-cascading into a crate already at a sufficient version appends a maintenance bullet to its existing changelog section instead of double-bumping. The .delta.toml Cargo.toml trip-wire is documented as a dependency of the release-deps CI gate so any version bump touches Cargo.toml -> skip=false. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Migrate the three remaining Invoke-GitCommand callsites in release-crate.ps1 (tag --list, git log, remote get-url) to the array-argument Invoke-Git wrapper in releasing.ps1, then delete the legacy wrapper. Removes the last use of Invoke-Expression-based git invocation in the release tooling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #436 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 332 332
Lines 24142 24142
=======================================
Hits 24142 24142 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Ralf Biedert (ralfbiedert)
approved these changes
May 21, 2026
Collaborator
|
Whops, I missed the |
The release-deps job previously depended on the delta job and used `delta.outputs.skip != 'true'` as a compute optimization to skip the analysis when no crate was affected. The optimization was structurally fragile - it relied on the implicit invariant that Cargo.toml and scripts/* remain in .delta.toml's trip_wire_patterns - and only saved a few seconds when there are no findings. Drop both the needs: [delta] dependency and the delta.skip gate. The check-unreleased-dependencies.ps1 script already exits fast (no findings -> no markdown, no comment) when there is nothing to report, so always running it is safe and simple. Also remove the corresponding NOTE in .delta.toml since that dependency no longer exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens the workspace crate release tooling by detecting “modified-but-unreleased” upstream workspace dependencies that could be missed when publishing to crates.io, and surfaces the same signal in CI via a new informational job.
Changes:
- Extracts shared release/dependency-graph and git helper logic into a dot-sourced PowerShell library (
scripts/lib/releasing.ps1). - Extends
scripts/release-crate.ps1to (optionally) scan for modified-but-unreleased upstream workspace deps after the existing downstream cascade, with interactive prompting to release them. - Adds a CI-only analyzer script and a new workflow job that posts/removes a sticky PR comment when such unreleased upstream dependency changes are detected.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/release-crate.ps1 | Adds base-ref/non-interactive options, refactors cascade flow, and performs a post-release upstream dependency scan. |
| scripts/lib/releasing.ps1 | New shared library providing safe git invocation, SemVer helpers, workspace metadata, and unreleased-dependency analysis. |
| scripts/check-unreleased-dependencies.ps1 | New CI companion script that emits a markdown report + step output for unreleased upstream dependency changes. |
| .github/workflows/main.yml | Adds release-deps job to run the CI analyzer and post/remove a sticky PR comment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- release-crate.ps1: drop -AllowFailure on the fetch in the base-ref resolver so the surrounding try/catch can actually trigger and emit a warning on fetch failure (previously the catch was unreachable because -AllowFailure returns $null instead of throwing). - check-unreleased-dependencies.ps1: route Get-RepoRoot through Invoke-Git instead of shelling out directly, matching the design described in the PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
martintmk
approved these changes
May 21, 2026
…rades them When the post-release dep scan runs a nested `Invoke-ReleaseFlow` for an upstream crate the user opts to release, the resulting cascade may upgrade a crate that was already in the release set (e.g., the initial release patch-bumped `foo` and the nested major release of an upstream dep now requires a major on `foo`). Previously the merge skipped duplicates outright, so `Show-ReleaseSummary` and the final `feat(crate): release v<version>` message reported a stale version that did not match what was actually written to `Cargo.toml`. Replace the skip-on-duplicate logic with an in-place update: keep the original `OldVersion` (the pre-PR baseline) and adopt the latest `NewVersion` from the nested cascade. New crates are still appended as before. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous logic compared the working tree against the PR base ref to decide which workspace crates had unreleased modifications. That missed a real scenario: an earlier PR merges a source change to `bytesbuf` without bumping its version, and a later PR bumps `bytesbuf_io` (which depends on `bytesbuf`). On crates.io the published `bytesbuf_io` resolves to the last released `bytesbuf`, which does *not* include the unreleased modification. Because the modification predates the PR's base ref, the old `BaseRef`- relative scan saw nothing to flag. Switch each crate's "modification baseline" to its own most-recent commit that touched `version =` or `publish =` in its `Cargo.toml`, derived via `git log -1 -G '^(version|publish)\s*='`. Any change under `crates/<folder>/` newer than that commit (committed, working-tree, or untracked) is treated as unreleased. `Get-CratesWithVersionBumps` (release-set detection) intentionally still diffs against the PR base ref — that's the correct anchor for "what is this PR releasing". Replaces `Get-GitFileChangeSet` / `Get-CratesWithFileChanges` with the new `Get-CrateLastReleaseBaseline` + `Get-CratesWithUnreleasedChanges` helpers and rewrites `Get-UnreleasedModifiedDependencies` to consume the per-crate modification map. Also persists the manual test plan as `scripts/RELEASE-DEPS-TEST-CASES.md` so future agents can re-run T1-T16 (original PR-vs-base coverage) and N1-N10 (multi-PR baseline coverage) when the logic changes. All N1-N9 scenarios were verified in a scratch worktree before this commit; N10 (brand-new crate) is structurally covered by the new-crate branch in `Get-CratesWithVersionBumps`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two real defects flagged on the latest review pass:
1. `check-unreleased-dependencies.ps1` was supposed to alphabetically sort
the release-set listing in the sticky PR comment, but the chain
`@(Get-CratesWithVersionBumps ...) | Sort-Object` silently broke. The
helper returns its HashSet via `Write-Output -NoEnumerate` so callers
can use `.Contains()`. That wrapping makes `Sort-Object` receive a
single object (the HashSet itself), so the sort is a no-op and the
foreach below iterates the HashSet in insertion order. Fixed by
unwrapping with `... | ForEach-Object { $_ }` before sorting.
2. `Add-CascadeBulletToVersionSection`'s `if ($subStart -ge 0)` branch
built the new file content with
`@($lines[0..($insertAt - 1)]) + @($bullet) + @($lines[$insertAt..($lines.Count - 1)])`.
When `$insertAt` equals `$lines.Count` (target sub-header is the last
content in the file, no bullets yet, no trailing blank lines), the
right-hand slice becomes `$lines[N..N-1]` which is a reverse-range that
silently aliases to the last element — so the last line was duplicated.
Reproduced on a synthesised changelog before fixing. Mirrored the EOF
guard that the `else` branch already has.
Verified by direct PowerShell repros for both: sort now yields
alphabetical order, EOF insertion no longer duplicates the sub-header
line, and the non-EOF + idempotency paths are unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Anticipates a future home for other script-related test material. The doc is unchanged; only its location moves. References inside the doc point at other scripts (`scripts/lib/releasing.ps1`, `scripts/release-crate.ps1`, `scripts/check-unreleased-dependencies.ps1`) from the repo root and remain valid after the move. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e set Documentation drift after the per-crate baseline refactor (cedd750). Four Copilot review comments on PR #436 flagged that synopses, parameter docs, and the user-facing PR-comment / interactive-warning text still said "modified vs the PR base ref" — but only the release-set anchor uses BaseRef; modifications are evaluated per crate against each crate's own last `version =` / `publish =` commit. Updated: - `scripts/check-unreleased-dependencies.ps1` — synopsis, description, BaseRef parameter docs, and the markdown body line ("unreleased modifications — changes newer than their last `version =` or `publish =` bump"). - `scripts/release-crate.ps1` — BaseRef parameter comment, the `Invoke-PostReleaseDepScan` function docstring, and the interactive warning text. (Switched the warning from a `"..."` to a `'...'` literal so the inline `` `version =` `` / `` `publish =` `` backticks aren't interpreted as PowerShell escape sequences — `` `v `` is vertical tab.) - `.github/workflows/main.yml` — `release-deps` job comment. Self-reviewed the rest of the PR diff for similar drift; the remaining "vs base" mentions (release-set BFS docstring, "Fetch base ref" workflow step name, "What this means" sticky-comment block) are legitimate and unchanged. Re-ran N1-N9 against the updated scripts in a scratch worktree: 9/9 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Capture the non-obvious lessons learned while building the N-series and T-series test harnesses, so future maintainers/agents who need to rebuild a harness from scratch don't have to relearn them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR #436 reviewer feedback: the It descriptions used `x`/`y` placeholders matching the local `0.x.y` Context label, but those collide with the standard SemVer convention (x=major, y=minor, z=patch). A reader skimming the test names could think 'breaking increments x' meant a major bump rather than the actual minor bump under Cargo's 0.x.y rules. Renamed the It strings to use 'minor component' / 'patch component' explicitly. Assertions are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Before: ``` running 1 test fetch future size: 536 bytes fetch_buffered future size: 2240 bytes fetch_text future size: 3552 bytes fetch_bytes future size: 2672 bytes fetch_json future size: 3104 bytes fetch_json_owned future size: 3536 bytes test http_request_builder::tests::fetch_future_sizes ... ok ``` After: ``` running 1 test fetch future size: 392 bytes fetch_buffered future size: 400 bytes fetch_text future size: 400 bytes fetch_bytes future size: 400 bytes fetch_json future size: 400 bytes fetch_json_owned future size: 400 bytes test http_request_builder::tests::fetch_future_sizes ... ok ```
…ependency and annotate Reply values * Sweep test scenarios, synthetic-workspace presets, and integration/unit tests to use 'dependent' / 'dependency' (and CamelCase / suffix variants) in place of 'upstream' / 'downstream', matching the terminology codified in docs/releasing.md. * Rename `UpDown5` synthetic-workspace preset to `FanInOut5` (and the associated 'updown5' TestDrive sub-path) so the preset name describes its topology in dependency-direction-neutral terms. * Rename `S01-clean-upstream-no-prompts.scenario.psd1` to `S01-clean-dependency-no-prompts.scenario.psd1` and update its `Name` field accordingly. * Annotate every numeric `Reply` value in scenario `Answers` sections with the menu-option label it triggers (e.g. `# Skip`, `# Non-breaking`, `# View diff`, `# Invalid (re-prompts)`) so scenario authors do not need to cross-reference release-flow.ps1 to understand the test. The single production-string assertion in PromptFlow.Tests.ps1 that matches 'downstream consumers may break' is left intact: that prose is emitted verbatim by `scripts/lib/release-flow.ps1` and is out of scope for this tests-only sweep. 367/367 Pester tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…from production scripts and docs
* Replace 'downstream consumers may break' with the simpler and equally
clear 'consumers may break' wherever it appears in user-facing warnings,
error messages, doc comments, help text, and the release docs.
* Reword the few internal comments that still used 'upstream'/'downstream'
in a pipeline-ordering sense ('upstream guard' -> 'earlier guard',
'downstream operations' -> 'later operations', etc.) so the directional
metaphor is no longer used anywhere in scripts/ or docs/releasing.md.
* Sync the corresponding PromptFlow.Tests.ps1 assertion to match the new
warning text.
The dependency-direction glossary in docs/releasing.md continues to
explicitly call out 'Avoid "upstream" / "downstream"' as the project's
naming rule; that meta-reference is the only remaining occurrence and
is intentional.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sander Saares (sandersaares)
added a commit
to microsoft/ox-tools
that referenced
this pull request
Jun 9, 2026
* feat(cargo-heather): recognize .psd1 and .psm1 as PowerShell files PowerShell data files (.psd1, used for module manifests and configuration hashtables) and PowerShell module files (.psm1) share the same `#` line-comment syntax as `.ps1` scripts but were silently skipped by the scanner because `FileKind::detect` only recognized the `.ps1` extension. This was visible downstream in microsoft/oxidizer#436, where a new suite of `.psd1` Pester scenario fixtures had to be hand-headered after a reviewer noticed they were missing MIT headers — `cargo heather` itself never flagged them because it never saw them. Extend `FileKind::detect` to classify `.psd1` and `.psm1` as `PowerShell`. Comment-style detection, header insertion, and fix logic all key off `FileKind::PowerShell` and need no other changes — the addition is purely an extension allowlist update. Also refresh `README.md` to list the currently-supported file kinds (this brought `.ps1`, Just, and `constants.env` along too — the README had drifted behind the implementation). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(cargo-heather): clarify PowerShell shebang exception and per-kind header extraction Address PR #38 review feedback: - `FileKind::PowerShell` doc: note that the `#` header is placed at the top OR after a leading `#!` shebang. Implementation (`checker::header_after_optional_shebang`) handles all three PowerShell extensions identically. - `README.md` 'How it works' step rewritten to describe header extraction per file kind (top-of-file for Rust/TOML/Just/env, after-shebang for PowerShell, inside frontmatter for cargo-script) instead of the misleading 'first comment block, `//` for Rust, `#` for the rest' shorthand. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(cargo-heather): split README How-it-works into one-liners + supported-kinds table Address PR #38 review feedback (ralfbiedert): the previous wrapped paragraph reading 'a // block at the top for Rust sources, a # block at the top for TOML / Just / constants.env, a # block after an optional #! shebang for PowerShell, and a # block inside the --- frontmatter for Rust cargo-scripts' was too dense for an at-a-glance description. Shortened the 'How it works' steps to one-liners and moved the per-file-kind comment style and header placement details into a new 'Supported file kinds' table, which doubles as a reference users can jump to. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evgenii (Vaiz)
approved these changes
Jun 9, 2026
Evgenii (Vaiz)
left a comment
Evgenii (Vaiz)
left a comment
Contributor
There was a problem hiding this comment.
I approve it with hope that this is a temporary solution that will be rewritten in something more maintainable when powershell
…tive includes * Drop the OXI_TEST_COMMON environment-variable indirection from Run-Tests.ps1; every *.Tests.ps1 file now dot-sources its shared helpers via `Join-Path $PSScriptRoot '..\_common\X.ps1'` (or `..\..\_common\X.ps1` for tests two levels deep under unit/). * Get-OxiRepoRoot in TestHelpers.ps1 likewise resolves the repo root from its own `` rather than the env var; the git-rev-parse fallback is no longer needed. This restores standalone runnability: `Invoke-Pester` on a single .Tests.ps1 file now works without first going through Run-Tests.ps1. `just test-scripts` still passes 367/367. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eraction The 'Skip package' label was misleading because it sounded like 'do not release this package', but in fact a cascade-toward-dependents walk can still pull the package into the release set even after the user picks option 2 — at which point the package is released with a cascade-derived change type and no separate per-package elevation prompt fires. Rename the menu option to `No material changes - release only if required by cascade logic` so the cascade interaction is explicit at decision time. Sync the Format-PackageMenu assertion in PromptFlow.Tests.ps1 and the `# Skip` cheat-sheet comments on `Reply = '2'` lines in the scenario fixtures to match the new label. 367/367 Pester tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sander Saares (sandersaares)
enabled auto-merge (squash)
June 10, 2026 10:54
Evgenii (Vaiz)
approved these changes
Jun 10, 2026
Kateřina Churanová (kate-shine)
pushed a commit
to kate-shine/oxidizer
that referenced
this pull request
Aug 14, 2026
Get-WorkspacePackages recorded every non-dev dependency in Deps under its package name alone, discarding the resolved path. Nothing downstream could then tell a workspace member from a registry crate or an out-of-workspace path dependency that happens to share its name. Every consumer of Deps asks a workspace-reachability question -- which crates can carry a released package's types into their own public API -- so a text-name match lets an unrelated external crate stand in for a workspace conduit and fabricate a path that does not exist. Consider a member `relay` that depends on the breaking target, and a candidate depending on a *different*, non-member `relay`. The candidate lands in the target's dependent closure, and the indirect-path predicate reports a path it does not have. If the candidate allowlists the target's own crate root, the renamed direct edge correctly rejects that root but the fabricated indirect path accepts it, and the planner assigns and propagates a breaking release nobody needs. Dependency identity now comes from the resolved path: a dependency joins the graph only when its `path` resolves to a member manifest directory. `path` is absent for a registry dependency and points outside crates/ for a non-member path dependency. Notes on shape: - The fix is at the loader rather than at the three cascade call sites, because all eight consumers of Deps already filter by workspace membership -- two of them with a literal `# external package` skip -- so they were all asking for this and matching on the wrong key. - DepAliases is filtered by the same guard. Leaving it unfiltered would keep the same conflation one level down, letting an external crate's `package = "..."` rename supply an allowlist root for a same-named member. - A member depended on through the registry rather than by path is now excluded, which is correct: bumping the local copy does not affect a consumer resolving to the published one. No such dependency exists in the workspace today -- all 58 members are depended on by path. The name-only match predates the exposure cascade; it dates to microsoft#436 and also affected the reverse closure and the direct-edge gate. Fixing the loader closes all of them at once. Mutation evidence: removing the path guard fails exactly the two tests that assert identity -- the non-member does not become an edge, and the candidate does not enter the target's dependent closure -- while the two controls still pass, so genuine member edges are not over-rejected. Suites: unit\release-packages 87/87, integration\DependencyRename 16/16, integration\ExposureCascade-RealWorkspace 14/14, integration\Releasing-Integration 44/44. New-SyntheticWorkspace grows an ExternalPackages spec key and an External dep flag, emitting a crate under external/ that the workspace excludes, since a non-member path dependency is the only shape that tells name matching apart from identity matching. Refs microsoft#665 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
When releasing a workspace package, an author may have also modified one of its workspace dependencies but forgotten to release that dependency. Locally everything builds via path-references, but once published the released package resolves against the last released version of each dependency on crates.io — missing the new changes. The dependency-side modification may even have landed in a previous PR that merged to main without a version increment, so a plain "diff vs PR base" check is not enough to catch it.
What changes for the user
Dependency review during release
release-packages.ps1now scans the dependency graph of every package in the release plan and surfaces every workspace dependency with unreleased modifications — evaluated per package against that dependency's own lastversion =/publish =commit, not against the PR base. For each finding the user can view the diff, skip the package, or release it alongside the original target.Multi-package releases are now atomic
Previously the script handled one package per invocation, with cascade decisions surfaced incrementally across re-runs. The new driver takes the entire release plan up-front:
After plan review, all writes —
Cargo.tomlversion increments,CHANGELOG.mdentries,README.mdregeneration, workspace[workspace.dependencies]updates — happen in one shot, followed by a workspacecargo check. Two guided alternatives are available when you don't have the full list ready up-front:-Changed(walk every package with unreleased modifications) and-All(walk every publishable package, modified or not).Semantic change type instead of numeric version components
The previous
-Bump major|minor|patchCLI conflated numeric version-string components with semantic intent — and they are not interchangeable: a breaking change on a0.xpackage moves the minor component, on a>=1.xpackage the major component. The new change-spec is a semantic label per package:breaking,nonbreaking, orpatch. Users who want to pin an exact target can do so explicitly —foo@1.0.0,bar@1.0.0-rc.1— with full SemVer 2.0 (three numeric components plus optional pre-release / build metadata) accepted.If the cascade computes a stronger change type than the user requested (e.g. the user said
patchbut the dependent exposes a breaking-changed dependency in its public API), the change type is auto-upgraded and the upgrade is shown in the plan display. Explicit version pins are not silently overridden — by default the plan is rejected; the new-Forceswitch overrides this when the pin really is intentional.Breaking changes for tooling muscle memory
scripts/release-crate.ps1is removed; usescripts/release-packages.ps1.-Bump major|minor|patchis gone; supply-Packages '<name>@<change-spec>', ...instead.Crate*→Package*symbol rename swept the release tooling (release-record property, function names, log strings). Cargo's own terminology (crates/,crates.io,Cargo.toml,cargo metadata) is preserved verbatim.Documentation and tests
Release vocabulary, workflow, and invariants now live in
docs/releasing.md;AGENTS.mdshrinks to a list of pointers. A Pester suite underscripts/tests/Pester/covers pure helpers, BFS / aggregation analyses on synthetic workspaces, and agent-driven end-to-end interactive scenarios. Runs locally viajust test-scriptsand on Windows + Linux in thescript-testsCI job.