Repository navigation
chore(deps): upgrade @effect/tsgo to 0.46.1 - #16360
Conversation
0.46.1 is the version Effect itself uses. It reports unknown rule names, so `importFromBarrel`, which Effect TSGo never ported, goes. It also extends `nodeBuiltinImport` to `crypto`, `timers`, and `stream`. Effect's Crypto covers only random values and ids, so each flagged import keeps Node's API behind a next-line opt-out that names what it needs.
github-actions
Bot
added
vouch:unvouched
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The dependency upgrade expands into a cross-cutting production migration involving hashing, service wiring, request retries, and timing behavior across multiple subsystems. It also adds numerous static-analysis suppressions and changes authentication-related files, warranting human review. You can add or adjust custom eligibility rules. Learn more. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates the ChangesEffect Diagnostic Updates
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Other Suggested reviewers: Merge Risk: ⚪ Minimal · up to The PR updates tooling configuration and adds scoped diagnostic suppressions without changing executable behavior. The focused check recognizes the directives and found no errors or warnings, leaving no actionable merge-blocking risk. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 7 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
vouch:trusted
|
@coderabbitai are all of these disable directives really true? effect/Crypto does have hashing. in the cases where we today use sync uuid gen, is there a reason for that or is it just leftovers? |
Effect's Crypto has `digest`, `randomBytes`, and UUIDs, so the opt-outs that said it has no `createHash` or `randomBytes` were wrong. Name only the APIs it lacks, and where a file needs only hashing or ids, say that the Node call is synchronous and Effect's is an Effect.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C2 'effect:|@effect/tsgo|effect/|nodeBuiltinImport' pnpm-workspace.yaml tsconfig.base.json
rg -n -C1 'effect-diagnostics-next-line.*nodeBuiltinImport' apps infra packages
rg -n -C3 '\b(randomUUID|createHash|createHmac|timingSafeEqual)\s*\(' apps/server/src apps/desktop/src packages/shared/src infra/relay/src
fd -i '^Crypto\.ts$|^package\.json$' | head -60Length of output: 77999 🏁 Script executed: #!/bin/bash
set -euo pipefail
python - <<'PY'
import urllib.request,re
url="https://unpkg.com/effect@4.0.1/src/Crypto.ts"
s=urllib.request.urlopen(url).read().decode()
print("SOURCE",url)
lines=s.splitlines()
for i,line in enumerate(lines):
if re.search(r'^export (const|interface|class|type)|hash:|randomUUIDv4:|randomBytes:|digest:|subtle\.|randomUUID\(',line):
print("\n".join(f"{j+1}: {lines[j]}" for j in range(max(0,i-3),min(len(lines),i+18))))
PY
for f in apps/server/src/assets/AttachmentUpload.ts apps/server/src/cli/app.ts apps/server/src/preview/Manager.ts apps/server/src/project/ProjectSetupScriptRunner.ts apps/server/src/vcs/GitVcsDriver.ts apps/server/src/mcp/McpHttpServer.ts apps/desktop/src/preview/Manager.ts; do
echo "=== $f ==="
ast-grep outline "$f" --items all --type function --view compact | head -90
done
rg -n -C12 'NodeCrypto\.randomUUID\(' apps/server/src/assets/AttachmentUpload.ts apps/server/src/cli/app.ts apps/server/src/preview/Manager.ts apps/server/src/project/ProjectSetupScriptRunner.ts apps/server/src/vcs/GitVcsDriver.ts apps/server/src/mcp/McpHttpServer.ts apps/desktop/src/preview/Manager.ts
rg -n -C3 'Crypto\.(digest|randomUUIDv4|randomBytes)|from "effect/Crypto"' apps infra packages | head -180Length of output: 2573 🏁 Script executed: #!/bin/bash
python - <<'PY'
import urllib.request,json,tarfile,io,re
meta=json.load(urllib.request.urlopen("https://registry.npmjs.org/effect/4.0.1", timeout=30))
t=tarfile.open(fileobj=io.BytesIO(urllib.request.urlopen(meta["dist"]["tarball"],timeout=30).read()),mode="r:gz")
for name in ["package/src/Crypto.ts","package/dist/Crypto.d.ts"]:
try: s=t.extractfile(name).read().decode()
except KeyError: continue
print("SOURCE",name)
lines=s.splitlines()
for i,line in enumerate(lines):
if re.search(r'^export (const|interface|class|type)|hash:|randomUUIDv4:|randomBytes:|digest:|subtle\.|randomUUID\(',line):
print("\n".join(f"{j+1}: {lines[j]}" for j in range(max(0,i-2),min(len(lines),i+23))))
break
PY
for f in apps/server/src/assets/AttachmentUpload.ts apps/server/src/cli/app.ts apps/server/src/preview/Manager.ts apps/server/src/project/ProjectSetupScriptRunner.ts apps/server/src/vcs/GitVcsDriver.ts apps/server/src/mcp/McpHttpServer.ts apps/desktop/src/preview/Manager.ts; do
echo "=== $f ==="
ast-grep outline "$f" --items all --type function | head -65
done
sed -n '140,205p' apps/server/src/assets/AttachmentUpload.ts
sed -n '175,225p' apps/server/src/cli/app.ts
sed -n '325,382p' apps/server/src/project/ProjectSetupScriptRunner.ts
sed -n '790,825p' apps/server/src/vcs/GitVcsDriver.ts
sed -n '315,345p' apps/server/src/mcp/McpHttpServer.ts
sed -n '1640,1685p' apps/desktop/src/preview/Manager.ts
sed -n '4175,4205p' apps/desktop/src/preview/Manager.ts
rg -n -C3 'Crypto\.(digest|randomUUIDv4|randomBytes)|from "effect/Crypto"' apps infra packages | head -180Length of output: 44002
I checked the published Hashing and other crypto operations
Some hash-only imports are clear migration candidates. For example:
Their hash calls already run inside Effect generators. Other hash calls belong to synchronous helpers, such as Streaming hashes need a separate assessment. Synchronous UUID generationI found no demonstrated requirement for synchronous UUID generation in the eight UUID-only files. Their UUID calls are already inside Effect execution:
The desktop calls include These look like migration leftovers rather than required synchronous boundaries. I did not check their history, so I cannot establish the original intent. There is a real type-level cost: My recommendation is to migrate the UUID-only and Effect-native hash-only cases. Keep narrow suppressions for unsupported operations, synchronous contracts retained intentionally, and incremental hashing. The comments should state those actual constraints instead of saying that Effect lacks hashing. ✏️ Learnings added
You are interacting with an AI system. |
|
You're right, I missed On sync UUIDs: mostly leftovers. Of the 13 Moving the Effect-context ones to |
|
The move is up in #16377. Two corrections to my reply above:
|
|
Autopilot could not be updated. Open Coding to check access and billing. |
# Conflicts: # apps/desktop/src/preview/Manager.ts # apps/server/src/assets/AttachmentUpload.ts # apps/server/src/assets/NativeAppIconResolver.ts # apps/server/src/cli/app.ts # apps/server/src/device/SshDeviceHost.ts # apps/server/src/mcp/McpHttpServer.ts # apps/server/src/preview/Manager.ts # apps/server/src/project/ProjectSetupScriptRunner.ts # apps/server/src/provider/ProviderCredentialStore.ts # apps/server/src/provider/openCodeUsageLimits.ts # apps/server/src/pullRequest/GitHubPullRequestCli.ts # apps/server/src/vcs/GitVcsDriver.ts # apps/server/src/ws.ts
github-actions
Bot
added
size:M
…s from main Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Replaces the nodeBuiltinImport opt-outs for synchronous createHash, randomUUID, randomBytes and Node timers with Effect's Crypto service and Effect.sleep/Schedule, threading Crypto through callers. Persisted hashes, refs and ids are unchanged; tests pin the previous outputs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
size:XL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reading the downloaded archive back to verify its SHA-256 loaded up to 1 GiB into memory. Hash each chunk as it streams with @noble/hashes, since Effect's Crypto only digests a whole buffer. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 participants
Problem
@effect/tsgois pinned at 0.41.0, behind the 0.46.1 that Effect itself uses. 0.41 accepts unknowndiagnosticSeveritykeys without a warning (Effect-TS/tsgo#747), which is howimportFromBarrelsilently stopped running after the move to TSGo.Change
@effect/tsgo0.41.0 → 0.46.1. From 0.46, an unknown rule name fails the typecheck.importFromBarrelcomes out oftsconfig.base.json, since TSGo never ported it. chore(lint): import Effect modules from their subpaths #16326 makes the identical change, so whichever merges first, the other still merges cleanly.nodeBuiltinImporttocrypto,timers, andstream. Instead of excepting what it flags, the code moves onto Effect's services:Crypto. @juliusmarminge's commits here move the remaining hashes and UUIDs, including synchronous helpers and tests. Helpers such ascheckpointRefForScopeOrdinal,remoteStateKey, andclaudePromptUuidbecome Effects that needCrypto.Effect.sleepinstead ofnode:timers/promises.@noble/hashes.Crypto.digestonly hashes a whole buffer, and archives can reach 1 GiB.Cryptolacks: HMAC, signing, key generation or import, ortimingSafeEqual. 16 are in tests, and one is a type-onlystreamimport.unstableApiUsage. It warns on every use ofeffect/http,process,reactivity, and the other unstable modules, about 5,800 times here.TS2790errors that look like a packaging regression.Scope and approval
A toolchain upgrade plus moving Node crypto and timer calls onto Effect services. No behavior change is intended;
@noble/hashesis the one new dependency. @juliusmarminge agreed to the upgrade in a private chat and wrote the second half of the migration. I work at CodeRabbit.Verification
JSON.stringifyfor plain strings, nested objects and arrays,undefinedfields, and non-finite numbers.