🐛 fix(release): a refusal names the bundle the operator passed, not the copy - #71
Conversation
`checkPlannedRelease` stages the bundle into a temporary directory to read it, and `ParseManifest` prefixes whatever file it was handed -- so a refusal named a `/tmp/morzer-plan-*` path the operator never chose and which is removed before they can look at it. - add `release.LoadManifestAs`, which names a source it did not read from - point the plan's manifest read at `--release`, as the run already does Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sabotaging `LoadManifest`'s source argument to an empty string degrades every manifest refusal to `error: : manifest is invalid:` and passed the entire suite -- nothing asserted that any path was named at all, on the path an operator hits most often. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The column carried `OPEN` on rows 1, 3, 4 and 5 at 6d3752d; answering each question overwrote its grade in place until the heading was the last evidence the column had ever held one. The answers move to a column of their own. Rows 1, 3 and 4 are LOCKED, rows 2 and 5 ASSUMED. The `rfc-index` gate goes from 27 problems to 22. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two entries and a departure from this wave's own plan, the RFC 0030 grading ruling, and a correction appended to wave 39 — the real `init` refuses a legacy bundle at exit 11 only when `--product` is passed. Drift count 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…packed The plan's half of this shipped one commit ago. `Resolve` extracts a `.tar.zst` into `morzer-resolve-*` before reading it, so a real install refused a legacy archive by naming a directory the operator never chose -- and the archive is the shape a vendor publishes. - add `release.LoadAs`, and thread the operator's path through `materialise` - document the contract on `LoadManifestAs`: an empty source is a caller bug Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A plan over an archive reads it through `Fetch`, which loads the bundle it just unpacked before the plan's own check gets to it -- so the directory case and the archive case leaked different temporary paths, and fixing one left the shape a vendor publishes still naming `morzer-plan-*`. Factors the archive fixture out of the test that built it inline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two entries from the self-audit pass: the archive half the first fix missed, and a `git checkout` that ate an uncommitted comment during the sweep. Carries the same defect in the remote sources, which needs a `ports.Ref` change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
the
size:L
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. Important Approval pendingCodeRabbit has no unresolved comments, but it could not review the latest commit because the review limit was reached. Follow the review guidance in this comment to continue. 📝 WalkthroughWalkthroughThe release loading path now accepts operator-supplied source names and uses them in errors. Local archive resolution and planned-release validation pass these names. RFC 0030 updates its Grade and Answer decisions table. ChangesRelease source names
RFC decision record
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Invalid bundle errors now identify the path supplied by the operator instead of a temporary copy, improving troubleshooting without changing bundle processing or installation behavior. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant LocalSourceAdapter
participant ReleaseLoader
participant ParseManifest
LocalSourceAdapter->>ReleaseLoader: LoadAs(directory, archive path)
ReleaseLoader->>ParseManifest: ParseManifest(manifest data, source name)
ParseManifest-->>ReleaseLoader: Parsed manifest or source-named error
ReleaseLoader-->>LocalSourceAdapter: Release or source-named error
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description gives a detailed summary, rationale, known gap, RFC context, and verification results. It does not complete the required Type, Design, Lifecycle invariants, Compatibility, Secrets & safety, or Risk & rollback sections. Resolution Complete the missing template sections. Mark the applicable Type and Design options, address lifecycle invariants, compatibility, secrets and safety, and document the risk and rollback path. Keep the reported verification commands and results in the required Verification section. Full details: Docstring CoverageExplanation Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
`Fetch` extracts an archive before it can validate it, so a bundle it refuses is one it has already written. Neither caller cleaned that up: `fetchRelease` returns straight out on a Fetch error, and `stepStageRelease`'s compensation keys off the release in engine state, which a failed Fetch never put there. An unusable release left in the store is one `update --to` away from being installed by somebody who never saw the error -- which is the reasoning the two sibling error paths in `fetchRelease` already carry. Reported on #71. Pre-existing; the validation itself is not new. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
1 participant
User description
What
A bundle that fails validation was refused by naming a temporary directory the operator never chose and which is removed before they can look at it.
ParseManifestprefixes the source so an author with several bundles open knows which one is being complained about. A path under/tmpanswers that question with one they cannot place, which is worse than no prefix at all.Four read-paths stage a copy before reading it. Three were leaking:
/tmp/morzer-plan-*/manifest.yaml--releasepath/tmp/morzer-plan-*/manifest.yaml/tmp/morzer-resolve-*/manifest.yamlThe rule across all three: the reader of a copy names the original.
release.LoadManifestAsandrelease.LoadAstake a source they did not read frommaterialisereturns what the operator namedFetchnames the archive it unpacked, not the destinationThe archive case matters most:
release.ManifestAt's own comment records that the archive is the shape a vendor publishes, so it is the primary install path.A coverage gap this found
Sabotaging
LoadManifest's source argument to an empty string degrades every manifest refusal toerror: : manifest is invalid:— and passed the entire suite. Nothing asserted that a refusal named any path at all.TestAFirstInstallRefusesADeprecatedBundlenow asserts it.TestAnInstallFromAnArchiveIsRefusedToowas also passing throughout: it asserted the refusal happened and never what it named.RFC 0030
Restores the Grade column, which held
OPENon rows 1, 3, 4 and 5 at6d3752d. Answering each question overwrote its grade in place, one edit at a time, until the heading was the last evidence the column had ever held one. Answers move to a column of their own; rows 1, 3 and 4 areLOCKED, rows 2 and 5ASSUMED.rfc_index.py checkgoes from 27 problems to 22.Also
logs/wave-39.mdgains a correction: the realinitrefuses a legacy bundle at exit 11 only when--productis passed. Without it the CLI reads the manifest to resolve the product name and refuses at exit 2, before any operation exists. The underlying cause —domain.ExitCodetestingErrCompensatedbeforeErrUsage, so the compensation wrapper outranks every cause — is recorded for an RFC rather than changed here.Verification
just ci— 86.7% (floor 84)just acceptance-cover— passedjust test-docker— passedParseManifest's prefix, and dropping the archive path. All killed.Known gap, carried
The remote sources have the same defect. An
httpsbundle whose manifest does not validate is refused with/tmp/morzer-download-*/bundle-0.tar.zst is not a valid manifest:and the URL appears nowhere.https.Resolvealready corrects this on the success path (resolved.Ref = ref) and leaves the failure path carrying the temp path.Not fixed here because it is a port change:
httpsdelegates by constructingports.Ref{Scheme: local.Scheme, Location: <temp>}, so there is nowhere to put the operator's name without adding one to the reference, and that re-opens the sharedReleaseSourceconformance battery for all three sources.🤖 Generated with Claude Code
CodeAnt-AI Description
Name the operator’s original bundle in validation errors
What Changed
Impact
✅ Clearer invalid-bundle errors✅ Actionable archive validation failures✅ No unusable temporary paths in operator output💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.