Make dev servers shareable from other devices - #4489

💭 I'm reviewing the TypeScript files that were changed to see how they align with the Effect service conventions, starting with the shared utilities and then moving through the development scripts and server configuration files. Looking at the imports and error handling patterns, I notice that DevSeedError in the dev-seed script extends plain Error rather than using Schema.TaggedErrorClass. But since seedDevDatabase is a synchronous function using node:sqlite rather than an Effect service, this might fall outside the scope of the conventions, which specifically apply when creating or refactoring Effect services. The CLI wrapper around it is worth examining though. Checking dev-share.ts, I see DevShareError properly uses Schema.TaggedErrorClass, which is good. However, the error's message and hint getters both use a lookup table keyed by the reason field to determine user-facing messages and hints. This is exactly the pattern the conventions warn against—when a reason field drives different messages and hints, those should be split into separate error classes instead of consolidated into one with conditional logic. This is a newly introduced error in the pull request, so it's a concrete violation worth flagging. The reason discriminator isn't just for internal diagnostics; it's actively choosing user-facing content, which crosses the threshold for needing separate error classes. to choose the user-facing message or control how callers handle the error. DevSeedTargetError and DevShareError both fit this pattern—they use a reason discriminator with switch/lookup logic in their message and hint getters to produce distinct user-facing messages. These are the clearest candidates for refactoring, though I need to verify they're actually Effect services before flagging them. I'm reconsidering whether these are genuine violations or if I should be more conservative. The convention does allow for a single error with multiple discriminator values when the failures share the same semantics, as long as the message is generic and derived from the operation. But here the messages are quite specific and different—different hints, different remediation steps—which suggests they might warrant separate error classes. Let me re-examine the convention more carefully to identify the clearest violations. Looking at the actual files... Looking at the DevShareError class, it uses a reason discriminator to determine the user-facing message and hint, which typically suggests splitting semantically distinct failures into separate error classes. However, the tests are checking the reason field directly with assertions like error.reason === "serve-failed", so in practice the reason is only being validated in tests rather than driving control flow in production code. Examining the existing error patterns in tailscale.ts shows separate classes for Spawn, Output, Exit, and Timeout errors, but those carry different fields. DevShareError's three reasons all share the same fields with only the detail varying, and both the message and hint are derived generically from the reason itself. This actually fits the pattern of keeping a single service-level error with a multi-value discriminator when failures share the same semantics and the messages are generic derivatives of the operation. The real question is whether tailscale-unavailable, no-tailnet-name, and serve-failed represent genuinely different failures or just variations of the same semantic failure ("couldn't share the dev server"). Since callers treat them uniformly—all logged as warnings with the dev server continuing—they seem to share the same semantic, which would support keeping them as one error. Now looking at DevSeedTargetError in scripts/dev-seed.ts with its three reasons: "shared-home", "missing-target", "not-a-worktree". Each has a distinct multi-line remediation message generated via switch, but callers don't branch on the reason—they just catch and log it. The failures share the semantic "cannot determine/use target", so this also fits the acceptable single-error pattern, though the switch-based message generation is starting to feel like it's pushing the boundaries of what should be one error. DevSeedError in scripts/lib/dev-seed.ts is a plain Error, not a TaggedErrorClass, but that's fine because it's thrown and caught synchronously in a non-Effect function (seedDevDatabase uses node:sqlite directly), then bridged into Effect at the CLI boundary. The convention applies to Effect service code, not synchronous utility functions. Now I need to check the auth.ts and serverRuntimeState.ts changes for service conventions, starting with NoRunningServerError in auth.ts. The code looks clean and follows conventions throughout. I'm checking the auth utilities, pairing grant store, and environment auth changes—they're mostly adding a purpose field and streamlining the session cookie resolution logic, nothing that violates the service patterns. Looking at this holistically, the PR is primarily focused on dev tooling rather than Effect service refactoring, so the conventions don't apply as heavily here. I've gone through the key files—dev-share.ts, dev-seed CLI, auth.ts, serverRuntimeState.ts, devHome.ts, devProxy.ts, and tailscale.ts—and found no clear violations. All imports use subpath namespaces, errors are properly structured with Schema.TaggedErrorClass and derived messages, and there's no fake DI or problematic runtime patterns. DevSeedError being a plain Error in synchronous code is acceptable, and the error discriminators in DevShareError and DevSeedTargetError follow a consistent single-error pattern. Now I'm verifying that no files have consolidated imports directly from "effect" that would break the convention. The consolidated imports from "effect" aren't in any of the modified files—they're only in untouched areas like the contracts preview and test files. After reviewing all the changed Effect service code, I can confirm it adheres to the conventions: namespace imports from subpaths, properly structured error classes with derived messages, and no hidden dependency injection or runtime issues. The error discriminators follow the acceptable pattern with multi-value operations and generic messages, and DevSeedError in the synchronous sqlite code is appropriately bridged at the Effect.try boundary. I'm not finding any concrete violations of the guidelines. There's a tension between the system requirement for valid JSON output and the instruction to respond with exactly "All clear" — but the schema takes precedence, so I'll structure the response as JSON with a success state and a concise summary indicating no issues found.