Skip to content

Validate package artifact source heads - #529

Merged
kentcdodds merged 1 commit into
mainfrom
cursor/investigate-empty-package-git-remote-e53e
Jun 8, 2026
Merged

kentcdodds merged 1 commit into
mainfrom
cursor/investigate-empty-package-git-remote-e53e

Conversation

@kentcdodds

@kentcdodds kentcdodds commented Jun 8, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Prevent package git remote minting from returning a Cloudflare Artifacts remote when the saved package source repo is missing or has no default-branch HEAD.
  • Validate package repo-session source HEADs before forking so sessions do not open from empty or stale source repos.
  • Add regression coverage for non-creating source HEAD lookup, package git remote minting, and repo session opening failures.

Validation

  • npx vitest run packages/worker/src/mcp/capabilities/packages/get-git-remote.node.test.ts packages/worker/src/repo/repo-session-do.node.test.ts packages/worker/src/repo/artifacts.node.test.ts
  • npm run format:check
  • npm run typecheck
  • npm run validate

Notes

  • No PR template was present in the repository.
Open in Web Open in Cursor 

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Enhanced validation for package sources to prevent operations when repository heads are missing or unavailable.
    • Improved error messages when package source repository information cannot be resolved.
    • Added checks during session creation to verify published package source repository states before proceeding.

Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 27815c02-4405-4a2c-8fc9-6394b5488a4e

📥 Commits

Reviewing files that changed from the base of the PR and between 209406a and f57e008.

📒 Files selected for processing (7)
  • packages/worker/src/mcp/capabilities/packages/get-git-remote.node.test.ts
  • packages/worker/src/mcp/capabilities/packages/get-git-remote.ts
  • packages/worker/src/repo/artifacts.node.test.ts
  • packages/worker/src/repo/artifacts.ts
  • packages/worker/src/repo/repo-session-do.node.test.ts
  • packages/worker/src/repo/repo-session-do.ts
  • packages/worker/src/repo/source-safety-policy.ts

📝 Walkthrough

Walkthrough

This PR adds published package source repository head validation to prevent opening sessions or minting git-remote capabilities when the artifact repo is unavailable or its default-branch HEAD does not match the published commit. The changes introduce resolveExistingArtifactSourceRepo and assertPublishedPackageSourceRepoHead helpers and integrate them into both get-git-remote and openSession flows with comprehensive test coverage.

Changes

Published package source repo head validation

Layer / File(s) Summary
Artifact repo existence checker
packages/worker/src/repo/artifacts.ts, packages/worker/src/repo/artifacts.node.test.ts
resolveExistingArtifactSourceRepo returns the repo handle when ready, null when not found, and throws for other states (importing/forking). resolveArtifactSourceHead updated to return a default head (branch: 'main', commit: null) when repo does not exist instead of attempting resolution.
Published package source head validation
packages/worker/src/repo/source-safety-policy.ts
New assertPublishedPackageSourceRepoHead function validates package sources by checking published_commit presence, resolving artifact source repo and default-branch HEAD, optionally enforcing HEAD commit matches published commit, and returning repo handle plus head metadata or null when not a package. Throws detailed recovery errors on validation failures.
get-git-remote capability integration
packages/worker/src/mcp/capabilities/packages/get-git-remote.ts, packages/worker/src/mcp/capabilities/packages/get-git-remote.node.test.ts
Handler refactored to call assertPublishedPackageSourceRepoHead instead of resolveArtifactSourceRepo().info() and derives remote/authenticated_remote and push setup from returned sourceHead. Rejects when no package source head available. Mock and test verify rejection when default-branch HEAD is null.
openSession published head validation
packages/worker/src/repo/repo-session-do.ts, packages/worker/src/repo/repo-session-do.node.test.ts
openSession validates published package source repo head with requirePublishedCommitHead:true before resolving sourceRepo for new sessions, using returned repo when present or falling back to resolveArtifactSourceRepo. Two new tests verify rejection when HEAD is null or commit mismatches published_commit, confirming fork callback is not invoked on pre-fork validation failure.

Possibly related PRs

  • kentcdodds/kody#525: Both PRs modify the package source "safety policy" plumbing used by package_get_git_remote: the main PR adds/uses assertPublishedPackageSourceRepoHead in get-git-remote.ts and extends source-safety-policy.ts, while the retrieved PR adds the restorable-snapshot guard assertRestorablePackageSourceSnapshot to get-git-remote.ts and expands source-safety-policy.ts with the underlying snapshot/overwrite checks.
  • kentcdodds/kody#352: The main PR's updates to the package_get_git_remote/Artifacts HEAD resolution flow and related packages/worker/src/repo/artifacts.ts helpers directly overlap with the retrieved PR's "direct Artifacts git push primitives" changes in the same MCP capability and artifact HEAD utilities.
  • kentcdodds/kody#509: Both PRs touch the getGitRemoteCapability minting flow in packages/worker/src/mcp/capabilities/packages/get-git-remote.ts—the main PR changes how remote/push are derived from published package source heads, while the retrieved PR adjusts the setup_commands git-config/fetch steps to also fetch refs/notes/*.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A rabbit hops through source repos with care,
Checking if heads exist, if branches are there—
No phantom commits in published domains,
Just validated paths and proper chains! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main purpose of the pull request: adding validation for package artifact source repository heads.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/investigate-empty-package-git-remote-e53e

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kentcdodds
kentcdodds marked this pull request as ready for review June 8, 2026 20:59
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🔎 Preview deployed: https://kody-pr-529.kentcdodds.workers.dev

Worker: kody-pr-529
D1: kody-pr-529-db
KV: kody-pr-529-oauth-kv

Mocks:

@kentcdodds
kentcdodds merged commit 6582c48 into main Jun 8, 2026
8 checks passed
@kentcdodds
kentcdodds deleted the cursor/investigate-empty-package-git-remote-e53e branch June 8, 2026 21:09
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