Repository navigation
Conversation
🦋 Changeset detectedLatest commit: bb926c2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@cloudflare/autoconfig
@cloudflare/build-output-utils
@cloudflare/codemods
@cloudflare/config
@cloudflare/containers-shared
create-cloudflare
@cloudflare/deploy-helpers
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-functions
@cloudflare/pages-shared
@cloudflare/runtime-types
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-plugin
@cloudflare/workers-auth
@cloudflare/workers-editor-shared
@cloudflare/workers-utils
wrangler
commit: |
686d9f1 to
64a5a33
Compare
|
Codeowners approval required for this PR:
Show detailed file reviewers
|
264a046 to
d64199e
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| await this.backend(repository, { action: "delete" }); | ||
| delete repositories[name.toLowerCase()]; | ||
| await this.saveRepos(repositories); |
There was a problem hiding this comment.
🔴 Failed cleanup strands a deleted repository
If Git cleanup fails, delete rejects after removing the repository from persistent metadata. A retry returns false, leaving its Git data inaccessible.
Learn more
The namespace stores repository metadata in a Durable Object, while a separate Git sidecar owns the repository directory. Saving the metadata removal before calling the sidecar makes an I/O failure in retireRepository leave the two stores out of sync. A failed rename leaves the Git directory at its original location, while a failed recursive removal can leave it in a retired directory. Either way, the caller receives an error but delete no longer finds the name for a retry.
Example: A Windows process temporarily holds a handle to repo.git; directory retirement fails with EPERM. The delete call rejects, but the next delete("repo") returns false and get("repo") fails while the original Git directory remains.
Recommended fix: Define a recoverable deletion state in LocalArtifactsNamespaceObject, or restore the metadata when the sidecar fails before renaming. For failures after a successful rename, finish deletion asynchronously or persist a pending-cleanup record so retries and startup can complete it without reporting an unsuccessful deletion of an already removed repository.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 9c03e0d: deletion commits metadata removal before touching the Git directory, so a failed metadata write leaves both metadata and contents intact. Git cleanup atomically retires the directory before removing files; restart cleans an interrupted retirement, and same-name creation handles an orphan left before retirement. Added metadata-failure and interrupted-cleanup regressions.
| function createRepoHandle( | ||
| namespace: LocalArtifactsNamespaceObject, | ||
| name: string | ||
| ): LocalArtifactsRepo { | ||
| const read = <T>(request: BackendRequest): Promise<T> => | ||
| publicCall(async () => | ||
| namespace.backend<T>(await namespace.getRepo(name), request) | ||
| ); |
There was a problem hiding this comment.
🟡 Deleted repository handles revive after recreation
After deletion and recreation under the same name, createRepoHandle resolves the replacement by name. A retained handle now reads or mutates the new repository.
Learn more
A repository handle stores only its name, and its operations call getRepo each time. Deleting the repository invalidates the handle temporarily, but recreating the name gives it access to the replacement with a different ID. This changes the identity of an existing object without a new get() call.
Example: Keep a handle for demo, delete demo, then create a fresh demo. Calling oldHandle.info() returns the new repository and oldHandle.createToken() creates a token for it.
Recommended fix: Bind each handle to the repository ID returned at get() time and check the ID on every operation; reject it after deletion or recreation.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 9c03e0d: a repository handle now captures its repository ID at get() and checks that ID for every read and mutation. The regression test retains a handle across delete/recreate and verifies it cannot inspect or mint tokens for the replacement.
| if (sourceUrl.protocol !== "https:") { | ||
| throw new ArtifactsError( | ||
| "INVALID_INPUT", | ||
| "Invalid source.url: must be an HTTPS URL." | ||
| ); | ||
| } | ||
| return { source, target, sourceUrl }; |
There was a problem hiding this comment.
🟥 Git imports reach private developer-network hosts
When a Worker supplies a private HTTPS URL, import() passes it to native Git without network restrictions. The Worker can probe services reachable from the developer's machine.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is an explicit HTTPS import requested by Worker code running in local development; private HTTPS Git origins are supported, and binding selection itself does not make a network request. We clarified that Git contacts hosts reachable from the developer machine, including private-network hosts, in the local development guide (SDK commit 9c03e0d and companion draft docs PR #34106). A general private-destination policy would need a separate product/security decision; this comment is not being represented as an automatic block or as deployed-service parity.
9c03e0d to
bb926c2
Compare
|
Closing this full-stack PR for now to avoid duplicate review while the split stack is reviewed. Part 1 is #16176; the simulator and development-tools branches remain pushed and will be proposed against cloudflare/workers-sdk:main separately. This PR and its review history are preserved and can be reopened if needed. No code was merged from this PR. |
2 participants
Summary
Add a Miniflare simulator for the public Artifacts binding contract, backed by native Git. It implements namespace/repository RPC, persistent namespace-scoped metadata and Git repositories, authenticated Git smart HTTP, and a per-Miniflare sidecar worker thread so synchronous Node binding proxies do not deadlock. Local development does not require Cloudflare credentials. An omitted
remoteorremote: falseuses local storage;remote: trueexplicitly selects remote bindings.This is a draft for contract and migration review, not a claim of production Git-engine or API parity. Native Git is a host dependency. An explicit HTTPS repository import can contact its specified Git source; local binding selection never implicitly synchronizes with production. The architecture diagrams and local setup guide are in
packages/miniflare/src/plugins/artifacts/.Original implementation by @GregBrimble. Adapted and integrated into Miniflare with additional contract and routing tests.
Local API surface
The simulator implements all 15 methods exposed by the Artifacts Worker binding. This is a surface inventory, not a claim of deployed-service behavioral parity.
create,get,list,import,deleteinfo,createToken,listTokens,revokeTokenreadBlob,readTree,readCommit,readFile,logforkgit-upload-pack,git-receive-packValidation checklist
Completed locally (macOS)
remoteandremote: false). Wrangler and Vite exercise loopback Git push and file reads; Wrangler checks reload, restart and listener shutdown. Workers Vitest checks create/get/list and a missing-file read. Explicitremote: truerouting remains covered by mocked tests, not live-service E2E.To do
never). The Vite fixture lint errors—shadowingfileURLToPathand using recursivermSyncinstead of the shared cleanup helper—are also fixed; theChecksjob passed.NODE_ENVwait timeout did not recur on the next full run. No unrelated fixture code was changed./index.jsfailure.ECONNRESETin a production-limit Local Explorer email fixture did not recur; its focused test also passes locally. No unrelated fixture code was changed.gitis acceptable for the initial release; adjust behavior and documentation to match those decisions.REST/dashboard emulation, billing/events, SSH and Git LFS are outside this PR's scope.