Skip to content

Support ctx.id.name in local dev for Durable Objects#6421

Merged
maxmcd merged 1 commit intomainfrom
mmcdonnell/dont-clear-id-name
Mar 26, 2026
Merged

Support ctx.id.name in local dev for Durable Objects#6421
maxmcd merged 1 commit intomainfrom
mmcdonnell/dont-clear-id-name

Conversation

@maxmcd
Copy link
Copy Markdown
Contributor

@maxmcd maxmcd commented Mar 25, 2026 •
edited
Loading

We now have support for ctx.id.name in Durable Objects. This PR removes the clearName() call in local dev that was simulating how the platform works by removing the name from the id. Now that we have support, we can keep the name.

The name is still missing from alarm invocations that wake up after the local isolate has shut down. I'll handle that work in a follow-up PR.

@maxmcd maxmcd requested review from a team as code owners March 25, 2026 21:28
ask-bonk[bot]
ask-bonk bot reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm Bonk, and I've done a quick review of your PR.

This PR removes the clearName() call that stripped ctx.id.name from Durable Object IDs in workerd's local server, making .name available during local dev for named DOs.

  1. [LOW] Test coverage gap for unique IDs. The updated test validates that ctx.id.name is a non-empty string for named DOs (idFromName), but there's no corresponding test asserting that ctx.id.name remains undefined for DOs accessed via newUniqueId(). Since the clearName code path previously applied to all kj::Own<ActorIdFactory::ActorId> (which includes both named and unique IDs), it would be good to confirm the unique-ID case is still correct after this change. The existing newUniqueId test at line 1855 doesn't check .name inside the DO constructor.

This review was generated by an AI assistant and may contain errors. Please verify any suggestions before applying them.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 25, 2026

Review posted successfully on PR #6421.

Summary: The PR cleanly removes the clearName() mechanism that was stripping ctx.id.name from Durable Object IDs in workerd's local server. The code change is straightforward and correct. I flagged one low-severity item: a test coverage gap where the complementary case (newUniqueId() DOs should still have undefined for .name) isn't tested. No high-severity issues found.

github run

@maxmcd maxmcd changed the title Remove clearName call on DO ctx.id for local dev Support ctx.id.name in local dev for Durable Objects Mar 25, 2026
@maxmcd maxmcd force-pushed the mmcdonnell/dont-clear-id-name branch from c0bb743 to fe1a2cf Compare March 25, 2026 22:07
shrima-cf
shrima-cf reviewed Mar 25, 2026
View reviewed changes
shrima-cf
shrima-cf reviewed Mar 25, 2026
View reviewed changes
@shrima-cf
Copy link
Copy Markdown
Contributor

shrima-cf commented Mar 25, 2026

Is there a way to test this?

@maxmcd maxmcd force-pushed the mmcdonnell/dont-clear-id-name branch from fe1a2cf to 0a6619a Compare March 25, 2026 22:46
@maxmcd
Copy link
Copy Markdown
Contributor Author

maxmcd commented Mar 25, 2026

@shrima-cf I added some more testing and left a clearer comment. Let me know what you think.

@shrima-cf
Copy link
Copy Markdown
Contributor

shrima-cf commented Mar 25, 2026

@shrima-cf I added some more testing and left a clearer comment. Let me know what you think.

LGTM. As discussed, add a test for the happy path

@maxmcd maxmcd force-pushed the mmcdonnell/dont-clear-id-name branch from 0a6619a to 76ac842 Compare March 26, 2026 03:36
@maxmcd maxmcd merged commit cce361e into main Mar 26, 2026
22 checks passed
@maxmcd maxmcd deleted the mmcdonnell/dont-clear-id-name branch March 26, 2026 19:27
@connyay connyay mentioned this pull request Apr 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ask-bonk ask-bonk[bot] ask-bonk[bot] left review comments

@shrima-cf shrima-cf shrima-cf approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@maxmcd @shrima-cf