Make artifacts scope optional - #109
Merged
Merged
Conversation
馃 Changeset detectedLatest commit: 0025615 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The scoping helpers took a session id as a required string, so there was no way to express a client that addresses the namespace as a whole. They now accept an undefined session id and pass names through untouched in both directions, filtering nothing. An unscoped name is held only to the charset the binding enforces, not to the stricter local-name rule. The names such a client works in are the ones the namespace stores, and those include the separator-bearing names scoped sessions mint, so rejecting the separator would put every scoped repository out of reach. Absence has to be spelled out. An empty string stays a validation error rather than a synonym for an omitted id, so an id that came out empty by accident fails loudly instead of quietly widening a client from one session to all of them.
createArtifact required a session id, so the only client it could build was one confined to a single session. A caller administering a namespace had no way to reach across it, and a Workspace with an Artifacts binding but no session id threw from its constructor. The session id is now optional. Omit it and the client addresses the namespace as a whole: names are the ones the binding stores, list returns every repository including those a session owns, and each one can be read, written to, or deleted under its stored name. Passing a session id behaves exactly as before. ArtifactClient.sessionId widens to string | undefined to say which of the two a client is. The scope is resolved once at construction, so a malformed id still fails there rather than on first use.
The CLI help stated the session-scoping contract unconditionally: names are local, a prefix is added on the way in, and repo list shows one session's repositories. None of that holds for a client built without a session id, and help is the contract a shell consumer reads before choosing a name. Each help path now takes the scoping the client actually applies and prints the paragraph that is true of it. The namespace-wide wording also spells out that the listing spans sessions and that a stored name reaches into one, so nobody has to infer the reach of the client they were handed.
A Workspace configured with an Artifacts binding and no session id threw from its constructor. The session id fell through to the empty string, and the facade rejected that, so the only way to get an artifacts surface at all was to have a session to scope it by. An absent session id now yields a client over the whole namespace. `artifacts.sessionId: null` asks for the same thing explicitly, for a workspace that carries a session id of its own but wants artifacts across every session. WorkspaceOptions.sessionId documents the empty string as its unset value, so an empty id is read here as "no session to scope by". The facade stays stricter and rejects an empty id outright: a caller reaching it directly has no such convention, and an id that came out empty by accident should not quietly widen a client from one session to all of them.
The artifacts documentation described one kind of client: bound to a session, working in local names, listing one session's repositories. A client built without a session id does none of that, and a reader following the old text would pick names that address the wrong repository or none at all. The interface spec gains a section on the namespace-wide client, covering what its names mean, that its reach includes writes and deletes, and why an empty session id is still an error rather than a way to ask for it. The package README, the docs index, and the module comments pick up the same qualification.
The line describing repo get promised metadata carrying a local name, which holds only for a client scoped to a session. An unscoped client returns the name the namespace stores.
The introduction said the wrapper is "session-scoped when you ask for it" and then described binding a binding, which took five lines to say what three say plainly. The paragraph on the empty session id opened with a drum roll and spent three lines on a rationale the code comments already carry. The changeset summary becomes one sentence and a pointer to the interface document, since it is read as a single line of release notes rather than as a description of the change.
"Facade" is a design-pattern name standing in for a plain word. Every prose use across the documentation, the package readmes, and the source comments becomes "wrapper", along with the one local variable in the computerd command-line tests that carried the term in its name. Three sentences needed more than the word swap to keep reading well: an observability comment that would have wrapped a span in a wrapper, a workspace comment whose wrappers wrapped their own entry points, and a project-layout line that listed the root export among the public entrypoints.
The artifacts wrapper rejects an empty session id so a blank value cannot quietly turn one tenant's client into a client over every repository in the namespace. The Workspace path read an empty id as an absent one instead, which walked around that check: a caller deriving the session id from an unset variable or a missing request field got the namespace-wide client rather than an error. An empty id now reaches the wrapper and fails there, as it did before the session id became optional. Omitting the id, or passing null, remains the way to ask for a client over the whole namespace.
aron-cf
force-pushed
the
artifacts-optional-scope
branch
from
August 18, 2026 13:50
5877e1b to
8aec189
Compare
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
An omitted property or an unset variable is undefined in JavaScript, so it intentionally selects the namespace-wide client. The Workspace comments and interface document incorrectly named those values as cases that raise InvalidSessionIdError. State the actual boundary instead: blank input and code that normalizes a missing value to an empty string fail, while omitted and undefined values use the documented default. Keep separate tests for an omitted property, an explicit undefined value, and an empty string so the distinction stays visible.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The disabled artifacts client carries an undefined session id, so the scope-aware help mistook it for a configured client over the whole namespace. Repository and token operations still failed, but their help promised access that did not exist. Give the dispatcher a third help state for a missing binding. Top level, repository, and token help now print the same configuration notice as failed operations instead of describing either session or namespace access. Direct artifacts clients keep their existing two states.
Comment on lines
+1352
to
+1356
| const configured = options.artifacts?.sessionId; | ||
| if (configured !== undefined) return configured; | ||
| return options.sessionId; | ||
| } | ||
|
|
There was a problem hiding this comment.
馃煥 Missing session id now silently grants namespace-wide artifacts access instead of failing
artifactsSessionId (packages/computer/src/workspace.ts:1352-1356) resolves to undefined whenever neither artifacts.sessionId nor WorkspaceOptions.sessionId is set, and createArtifact then builds an unscoped client. An unscoped client takes stored names literally and applies no prefixing or filtering (packages/computer/src/artifacts/scope.ts:96-98, packages/computer/src/artifacts/scope.ts:115-116), so list, get, createToken, and delete reach every repository in the namespace, including those other tenants own. Before this change the same configuration threw at construction, so a workspace that forgot to pass a session id failed loudly; now it comes up with namespace-wide authority. Because the worker-shell backend always exposes the artifacts command and forwards to workspace.artifacts.cli(...), an agent running in that shell of a session-less workspace can enumerate, mint write tokens for, and delete other sessions' repositories.
Was this helpful? React with 馃憤 or 馃憥 to provide feedback.
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
1 participant
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.
The artifacts client always needed a session id. That was fine for an agent that owns one corner of a namespace, but it left no way to look at the namespace as a whole, and a
Workspaceconfigured with an Artifacts binding and no session id threw from its constructor rather than giving you a client at all.The session id is now optional. Leave it out and the client works in the names the namespace stores:
list()returns every repository, including the ones sessions own under<session>__<name>, and each of those can be read, written to, or deleted by that name. Pass a session id and nothing changes. AWorkspacepicks the session up from its ownsessionId, andartifacts: { binding, sessionId: null }asks for the namespace-wide client even when the workspace has a session of its own.That reach covers writes and deletes, so the unscoped client is for a caller that administers a namespace, not one that lives in it. To keep the two apart, an empty session id stays an error: only an omitted or
nullid means "no session", so an id that came out blank by accident fails instead of quietly opening up every session.Names get one concession. A scoped client still refuses
__in a name, since that separator is what makes the split unambiguous. An unscoped client has to accept it, because the stored names it works in contain it.The scoping rules, the client, and the workspace wiring each landed with their own tests, and the in-shell
artifactscommand now prints help that matches the client it is bound to instead of promising a session prefix that may not be applied. Runnpm test --workspace @cloudflare/computerto see the lot.Two documentation notes.
docs/15_artifacts_interface.mdgained a section on the namespace-wide client, covering what its names mean and why a blank id is rejected. Separately, the word "facade" is gone from the prose across the repository in favor of "wrapper", which says the same thing without borrowing a pattern name.