Fix package artifact rebuild diagnostics - #560
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughPublished package artifact targets are extracted into a shared module, repo checks consume those targets for bundle validation, and rebuild/publish error handling now includes nested cause chains and transient Durable Object reset retries. ChangesPublished package artifact target and publish error flow
Sequence Diagram(s)sequenceDiagram
participant publishExternalPushCapability
participant isTransientDurableObjectResetError
participant rebuildPublishedPackageArtifactsViaRepoSession
participant rebuildPublishedPackageArtifact
publishExternalPushCapability->>rebuildPublishedPackageArtifactsViaRepoSession: start external publish rebuild
rebuildPublishedPackageArtifactsViaRepoSession->>rebuildPublishedPackageArtifact: rebuild target
rebuildPublishedPackageArtifact-->>rebuildPublishedPackageArtifactsViaRepoSession: Error(cause chain)
rebuildPublishedPackageArtifactsViaRepoSession-->>publishExternalPushCapability: throw rebuild failure
publishExternalPushCapability->>isTransientDurableObjectResetError: inspect nested cause chain
isTransientDurableObjectResetError-->>publishExternalPushCapability: true
publishExternalPushCapability->>publishExternalPushCapability: warn transient Durable Object reset
publishExternalPushCapability->>rebuildPublishedPackageArtifactsViaRepoSession: retry external publish flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
kentcdodds
marked this pull request as ready for review
June 25, 2026 12:24
Contributor
|
🔎 Preview deployed: https://kody-pr-560.kentcdodds.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/mcp/capabilities/repo/package-artifact-rebuild.ts (1)
5-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated error-cause helpers across capabilities.
getErrorCausehere is identical to the one inpackages/worker/src/mcp/capabilities/packages/publish-external-push.ts(Lines 31-36), andformatErrorCauseChainshares the sameseen-guardederror -> causetraversal as that file'serrorCauseChainIncludes. Since both files participate in the same publish/rebuild flow, consider extracting a sharedgetErrorCause+ a single cause-chain walker (e.g. alongsideerror-message.ts) so the traversal/cycle-guard logic stays consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/mcp/capabilities/repo/package-artifact-rebuild.ts` around lines 5 - 22, The error-cause traversal helpers in getErrorCause and formatErrorCauseChain are duplicated and should be shared with the similar logic in publish-external-push.ts. Extract a common getErrorCause plus a single cause-chain walker/helper near the existing error-message utilities, then update package-artifact-rebuild.ts to use it so the seen-guarded error->cause traversal stays consistent across both capabilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/src/mcp/capabilities/repo/package-artifact-rebuild.ts`:
- Around line 5-22: The error-cause traversal helpers in getErrorCause and
formatErrorCauseChain are duplicated and should be shared with the similar logic
in publish-external-push.ts. Extract a common getErrorCause plus a single
cause-chain walker/helper near the existing error-message utilities, then update
package-artifact-rebuild.ts to use it so the seen-guarded error->cause traversal
stays consistent across both capabilities.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 62573319-8b3b-4f33-b24a-cd779b616dc7
📒 Files selected for processing (8)
packages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.tspackages/worker/src/mcp/capabilities/packages/publish-external-push.tspackages/worker/src/mcp/capabilities/repo/package-artifact-rebuild.tspackages/worker/src/package-runtime/package-artifact-targets.tspackages/worker/src/package-runtime/published-bundle-artifacts.node.test.tspackages/worker/src/package-runtime/published-bundle-artifacts.tspackages/worker/src/repo/checks.node.test.tspackages/worker/src/repo/checks.ts
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
1 of 2 tasks
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
2 participants
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.
Summary
package_publish_external_pushvalidates the same module/importable/app targets it will later persist.package_publish_external_pushwhen a transient Durable Object reset is wrapped inside an artifact rebuild error cause.Validation
npm test -- --run packages/worker/src/repo/checks.node.test.ts packages/worker/src/package-runtime/published-bundle-artifacts.node.test.ts packages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.tsnpm run format:checknpm run lint(0 errors; existing unrelated warnings only)npm run typechecknpm run validate(latest local run: 120 test files / 386 tests passed)Production / operations
No schema migration is required. This prevents future publishes from advancing source state when persisted bundle artifacts would fail to build, and makes any rebuild failure identify the exact target/cause. For an already-stuck production package whose
published_commitwas advanced without usable artifacts, re-runpackage_publish_external_pushafter the package source is corrected; if the commit itself is invalid for persisted module artifacts, publish a corrected commit or manually restoreentity_sources.published_committo the last known-good commit and re-run publish.Summary by CodeRabbit
New Features
Bug Fixes