Adopt no-ambient-nondeterminism: route time, ids, and randomness through Effect seams - #9
Merged
Merged
Conversation
Ambient Date.now() and SessionId.create() bypassed the Clock and Ids service seams the rest of Fold already uses, so session-layout writes and id minting could not be pinned in tests. - The SessionLayout deletion tombstone and ViewedChanges now read Clock.currentTimeMillis instead of Date.now(). - prepareSessionLog mints via the Ids service; the requirement threads up the launch path and the app runtime provides layerLiveIdFactory (cli.ts). The TUI acquire sites and the ConfigAgent example provide the same live layer. - The Launch and SessionLayout tests shadow the launch/prepare helpers with the live id layer, preserving behavior while leaving DeterministicIds as a seam. Behavior-preserving prep for the no-ambient-nondeterminism rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a0179a-5c6f-7ab7-8e1e-92b5a54ffad3
writeConfig's atomic-rename temp path and BashTool's spill filename used Date.now() and node crypto randomBytes directly. Route them through the Clock and Random default services so tests can pin the generated names. Runtime behavior is unchanged - Random defaults to Math.random, which is fine for filename uniqueness (these are not security tokens). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a0179a-5c6f-7ab7-8e1e-92b5a54ffad3
PKCE verifier/state/nonce and the S256 challenge digest used the ambient crypto.getRandomValues / crypto.subtle globals, so their values could not be pinned in tests. Route them through Effect's Crypto service (randomBytes + digest); the live NodeCrypto layer is node:crypto - a real CSPRNG - and is already provided app-wide via NodeServices at cli.ts. - generatePkce/generateState (codex) and generateXaiPkce + state/nonce (xai) now yield Crypto; crypto failures orDie, matching the prior Effect.promise defect semantics, so only the R channel changes. - The browser-flow call sites provide NodeCrypto.layer locally, keeping the auth service interfaces at R = never (no consumer or TUI ripple). Security is unchanged (same node CSPRNG). Tests can now provide a deterministic Crypto layer to pin PKCE values. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a0179a-5c6f-7ab7-8e1e-92b5a54ffad3
Vendor the TypeOnce no-ambient-nondeterminism rule into tools/oxlint/automation (adapted: dropped the .tsx allow-list option, mapped messages to messageIds; the scope analysis is kept so local bindings like `const crypto = yield* Crypto.Crypto` pass while the ambient globals are caught) and enforce it as an error. Ambient Date.now()/new Date()/Math.random() and the global crypto object must go through Effect's Clock, Random, and Crypto capabilities. TUI/theme and test files are excluded via overrides. Zero in-scope findings remain after the preceding seam refactors (session time/ids, temp paths, OAuth CSPRNG). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a0179a-5c6f-7ab7-8e1e-92b5a54ffad3
Chaining Effect.provide(layerLiveIdFactory) after Effect.provide(NodeServices.layer) tripped the effect tsgolint multipleEffectProvide check (TS377033), failing typecheck. Merge both layers with Layer.mergeAll and provide once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a0179a-5c6f-7ab7-8e1e-92b5a54ffad3
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.
What
Adopts
automation/no-ambient-nondeterminism(vendored from the TypeOnce catalog) and clears every in-scope finding first, so ambient time and randomness flow through Effect'sClock,Random,Ids, andCryptoseams — mockable in tests instead of reading wall-clock /Math.random/ globalcryptodirectly.The seams
Date.now()in the deletion tombstone + viewed-changes writesClock.currentTimeMillis(default service)SessionId.create()inprepareSessionLogIdsservice; requirement threads tocli.ts, which provideslayerLiveIdFactoryDate.now()/ noderandomBytesClock+Random(default services)crypto.getRandomValues/crypto.subtleCryptoservice (randomBytes+digest); liveNodeCryptolayer already provided viaNodeServicesNotes / decisions
Clock/Randomare default services (noRchange;DeterministicClockalready mocks Clock). TheIdsrequirement threads to the app runtime (cli.ts), matching the existingDeterministicIdstest layer. Tests shadowlaunchSession/prepareSessionLogwith the live id layer, so no assertions shift.NodeCryptoisnode:crypto— a real CSPRNG (Effect's generalRandomisMath.random-backed, so it is not used for tokens). The browser-flow call sites provideNodeCrypto.layerlocally, keeping the auth service interfaces atR = never(no consumer/TUI ripple). A test can now provide a deterministicCryptolayer to pin PKCE values.const crypto = yield* Crypto.Cryptopasses while ambient globals are caught.new Date(knownValue)(pure formatting) is not flagged. TUI/theme and test files are excluded via overrides.Testing
bun run typecheck,bun run lint,bun run format:checkall clean. Full test suite passes exceptfold-codex/CodexLive.vi.test.ts(2 live-network tests,429 usage_limit_reached, CI-skipped and unrelated).🤖 Generated with Claude Code